netdev - Re: [PATCH] FDDI: defxx: simplify if-if to if-else

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.2204250009240.9383@angie.orcam.me.uk>
Date:   Mon, 25 Apr 2022 00:26:10 +0100 (BST)
From:   "Maciej W. Rozycki" <macro@...am.me.uk>
To:     Andrew Lunn <andrew@...n.ch>
cc:     Wan Jiabing <wanjiabing@...o.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, kael_w@...h.net
Subject: Re: [PATCH] FDDI: defxx: simplify if-if to if-else

On Mon, 25 Apr 2022, Andrew Lunn wrote:

> >  NAK.  The first conditional optionally sets `bp->mmio = false', which 
> > changes the value of `dfx_use_mmio' in some configurations:
> > 
> > #if defined(CONFIG_EISA) || defined(CONFIG_PCI)
> > #define dfx_use_mmio bp->mmio
> > #else
> > #define dfx_use_mmio true
> > #endif
> 
> Which is just asking for trouble like this.
> 
> Could i suggest dfx_use_mmio is changed to DFX_USE_MMIO to give a hint
> something horrible is going on.

 There's legacy behind it, `dfx_use_mmio' used to be a proper variable and 
references were retained not to obfuscate the changes that ultimately led 
to the current arrangement.  I guess at this stage it could be changed to 
a function-like macro or a static inline function taking `bp' as the 
argument.

> It probably won't stop the robots finding this if (x) if (!x), but
> there is a chance the robot drivers will wonder why it is upper case.

 Well, blindly relying on automation is bound to cause trouble.  There has 
to be a piece of intelligence signing the results off at the end.

 And there's nothing wrong with if (x) if (!x) in the first place; any 
sane compiler will produce reasonable output from it.  Don't fix what 
ain't broke!  And watch out for volatiles!

  Maciej