Re: [Qemu-devel] [PATCH v2 2/8] usb: a trivial code change for more idiomatic writing style

[Markus Armbruster]

Andreas Färber <address@hidden> writes:

> Am 01.08.2014 05:32, schrieb Gonglei (Arei):
>> Hi,
>> 
>>> Subject: Re: [PATCH v2 2/8] usb: a trivial code change for more
>>> idiomatic writing
>>> style
>>>
>>> On 07/31/2014 08:32 PM, address@hidden wrote:
>>>> From: Gonglei <address@hidden>
>>>>
>>>> Signed-off-by: Gonglei <address@hidden>
>>>> ---
>>>>  hw/usb/dev-audio.c | 2 +-
>>>>  hw/usb/dev-mtp.c   | 4 ++--
>>>>  hw/usb/hcd-ehci.c  | 2 +-
>>>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
>>>> index bfebfe9..988f6cc 100644
>>>> --- a/hw/usb/dev-audio.c
>>>> +++ b/hw/usb/dev-audio.c
>>>> @@ -371,7 +371,7 @@ static void output_callback(void *opaque, int avail)
>>>>              return;
>>>>          }
>>>>          data = streambuf_get(&s->out.buf);
>>>> -        if (NULL == data) {
>>>> +        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...

!strcmp() is somewhat error prone, because it suggests inequality.
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.
data == 0 is right out.

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.