On 10/24/2011 06:42 PM, Marek Ol??k wrote:
> On Sat, Oct 8, 2011 at 1:32 PM, Thomas Hellstrom<thomas at shipmail.org>
> wrote:
>
>> On 10/08/2011 01:27 PM, Ville Syrj?l? wrote:
>>
>>> On Sat, Oct 08, 2011 at 01:10:13PM +0200, Thomas Hellstrom wrote:
>>>
>>>
>>>> On 10/08/2011 12:26 PM, Ville Syrj?l? wrote:
>>>>
>>>>
>>>>> On Fri, Oct 07, 2011 at 10:58:13AM +0200, Thomas Hellstrom wrote:
>>>>>
>>>>>
>>>>>
>>>>>> Oh, and one more style comment below:
>>>>>>
>>>>>> On 08/07/2011 10:39 PM, Marek Ol??k wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> +enum ttm_buffer_usage {
>>>>>>> + TTM_USAGE_READ = 1,
>>>>>>> + TTM_USAGE_WRITE = 2,
>>>>>>> + TTM_USAGE_READWRITE = TTM_USAGE_READ | TTM_USAGE_WRITE
>>>>>>> +};
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> Please don't use enums for bit operations.
>>>>>>
>>>>>>
>>>>>>
>>>>> Now I'm curious. Why not?
>>>>>
>>>>>
>>>>>
>>>>>
>>>> Because it's inconsistent with how flags are defined in the rest of the
>>>> TTM module.
>>>>
>>>>
>>> Ah OK. I was wondering if there's some subtle technical issue involved.
>>> I've recently gotten to the habit of using enums for pretty much all
>>> constants. Just easier on the eye IMHO, and avoids cpp output from
>>> looking like number soup.
>>>
>>>
>>>
>> Yes, there are a number of advantages, including symbolic debugger output.
>> If we had flag enums that enumerated 1, 2, 4, 8 etc. I'd feel motivated to
>> move
>> all TTM definitions over.
>>
> I don't think that how it is enumerated matters in any way. What I
> like about enums, besides what has already been mentioned, is that it
> adds a self-documentation in the code. Compare:
>
> void ttm_set_bo_flags(unsigned flags);
>
> And:
>
> void ttm_set_bo_flags(enum ttm_bo_flags flags);
>
> The latter is way easier to understand for somebody who doesn't know
> the code and wants to implement his first patch. With the latter, it's
> clear at first glance what are the valid values for "flags", because
> you can just search for "enum ttm_bo_flags".
>
> I will change the enum to defines for the sake of following your code
> style convention, but it's an unreasonable convention to say the
> least.
>
> Marek
>
I'm not going to argue against this, because you're probably right.
The important thing is that we get the fix in with or without enums.
Thanks,
Thomas
