All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@orcam.me.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Wan Jiabing <wanjiabing@vivo.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	kael_w@yeah.net
Subject: Re: [PATCH] FDDI: defxx: simplify if-if to if-else
Date: Mon, 25 Apr 2022 00:26:10 +0100 (BST)	[thread overview]
Message-ID: <alpine.DEB.2.21.2204250009240.9383@angie.orcam.me.uk> (raw)
In-Reply-To: <YmXMcUAhUg/p1X3R@lunn.ch>

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

  reply	other threads:[~2022-04-24 23:26 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-24  9:28 [PATCH] FDDI: defxx: simplify if-if to if-else Wan Jiabing
2022-04-24 10:39 ` Maciej W. Rozycki
2022-04-24 22:17   ` Andrew Lunn
2022-04-24 23:26     ` Maciej W. Rozycki [this message]
2022-04-25  3:27       ` Jiabing Wan
2022-04-25 12:06       ` Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.2204250009240.9383@angie.orcam.me.uk \
    --to=macro@orcam.me.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=kael_w@yeah.net \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=wanjiabing@vivo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.