On 08/01/2014 12:41 AM, Markus Armbruster wrote:
>>>>> + if (data == NULL) {
>>>>
>>>> Wouldn't it be even more idiomatic as:
>>>>
>>>> if (!data) {
>>>>
>>>> Probably applies throughout your series.
>>>>
>>> OK, will do. Thanks!
>>
>> Not so quick! You are free to use that in your patches, but please don't
>> change all code that way without the author's consent. Just like "equals
>> null" is a natural English way of reading, compared to "null equals
>> something", "not null" reads like a boolean expression to me, and even
>> worse while all valid C, "not strcmp" leads to mind-boggling inverted
>> logic...If it's going to be controversial, then the right thing to do is that both '(!ptr)' and '(ptr == NULL)' are acceptable, and that preference should be given to consistency to nearby code. > > !strcmp() is somewhat error prone, because it suggests inequality. That's why libvirt uses a STREQ() macro; it evaluates to !strcmp under the hood, but it's a LOT easier to reason about 'STREQ(a, b)' than it is to reason about '!strcmp(a, b)'. > Can't claim that for !data. That one suggests "no data", which is > exactly right. Like Eric, I prefer it to the cumbersome data == NULL. I also prefer 'if (ptr)' over 'if (ptr != NULL)'. > data == 0 is right out. Correct, especially if data is a pointer (hmm, our coding style should mention that we STRONGLY prefer NULL over 0 when referring to the null pointer). > > Since there's no consensus on !data vs. data == NULL, you're free to > follow your own taste in new code. When changing existing code, imitate > nearby code. When nearby code is inconsistent, it's your own taste > again. Yes, so capturing this sentiment in the coding style guide would be worthwhile. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
