Messages in this thread | | | Date | Mon, 25 Apr 2022 14:06:40 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH] FDDI: defxx: simplify if-if to if-else |
| |
On Mon, Apr 25, 2022 at 12:26:10AM +0100, Maciej W. Rozycki wrote: > 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.
Yes, something like that would be good.
> > 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.
Unfortunately, there are a number of bot drivers who do blindly rely on automation. We have had to undo the same broken bot driven changes a few times, and ended up adding extra comments to catch the eye of the bot drivers.
Andrew
| |