Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Eric Blake]

On 12/14/19 7:38 AM, Philippe Mathieu-Daudé wrote:
> On 12/14/19 3:52 AM, Finn Thain wrote:
> > On Fri, 13 Dec 2019, address@hidden wrote:
> >
> > > === OUTPUT BEGIN ===
> > > 1/10 Checking commit 9c9ffc38e9b9 (dp8393x: Mask EOL bit from 
> > > descriptor addresses)
> > > ERROR: return is not a function, parentheses are not required
> > > #24: FILE: hw/net/dp8393x.c:200:
> > > +    return (s->regs[SONIC_URDA] << 16) | (s->regs[SONIC_CRDA] & 
> > > 0xfffe);
> > >
> > > ERROR: return is not a function, parentheses are not required
> > > #33: FILE: hw/net/dp8393x.c:220:
> > > +    return (s->regs[SONIC_UTDA] << 16) | (s->regs[SONIC_TTDA] & 
> > > 0xfffe);
> >
> > I expect that checkpatch.pl has no idea about operator precedence, but
> > these parentheses could actually be omitted.

You are correct: It's a false positive; you can safely ignore it.

> > I kept them because I don't want readers to have to remember that bit
> > shift operator has higher precedence than bitwise OR operator, or look it
> > up if they don't.
> >
> > The existing code also has those unnecessary parentheses.
> >
> > Please let me know if this patch should include a code style change.
>
> This is a bug in checkpatch. Since this script doesn't have dedicated 
> maintainer, I Cc'ed the recent contributors:

However, it's complex enough, and the false positive occurs infrequently 
enough, that just ignoring it (instead of trying to patch checkpatch) is 
also fine, and probably what will happen.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org