Re: [RFC 1/7] mac80211: mesh power mode indication in QoS frames

[Johannes Berg <johannes@sipsolutions.net>]

On 10/23/2011 3:10 PM, Ivan Bezyazychnyy wrote:
> On 21 October 2011 13:52, Johannes Berg<johannes@sipsolutions.net>  wrote:
>> On Fri, 2011-10-21 at 12:36 +0400, Ivan Bezyazychnyy wrote:
>>
>>> +/* mesh power save level subfield mask */
>>> +#define IEEE80211_QOS_CTL_PS_LEVEL      0x0200
>>
>> That looks like it could use a better name?
>
> Do you mean something like IEEE80211_QOS_CTL_MESH_POWER_SAVE_LEVEL?
> The first half is similar to previous constants.

Not really .. I mean, it's like you have two levels, 0 and 1, but you 
say this is the level. Why not say this is the 1 level, and the 0 level 
is implied otherwise?

For example, typically, if we have something like a bit in a field that 
indicates the device is getting too hot, we'll call it
FOOBAR_DEVICE_TOO_HOT
and not
FOOBAR_DEVICE_TEMPERATURE_LEVEL

Basically I think that the constant name you have now tells me nothing 
about what it means when the bit is set and I think it'd be better if it 
did. That might not even be possible, but I think in that case you 
really should add a comment, e.g.:

/*
  * mesh power save level:
  * set - the device is doing ...
  * unset - the device is doing ...
  */
#define ...

johannes