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

[Gonglei (Arei)]

Hi,

> Subject: Re: [Qemu-devel] [PATCH v2 2/8] usb: a trivial code change for more
> idiomatic writing style
> 
> 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. 

Agreed.

> When changing existing code, imitate
> nearby code.  When nearby code is inconsistent, it's your own taste
> again.

Yes, I think this is a pretty good policy. Thanks!

Best Regards,
-Gonglei