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

[Maciej W. Rozycki]

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