Re: [PATCH] staging: wlan-ng: Avoid bitwise vs logical OR warning in hfa384x_usb_throttlefn()

From: Nathan Chancellor <nathan@kernel.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev
Subject: Re: [PATCH] staging: wlan-ng: Avoid bitwise vs logical OR warning in hfa384x_usb_throttlefn()
Date: Fri, 15 Oct 2021 10:13:05 -0700	[thread overview]
Message-ID: <YWm2oVk9YKzjhYYi@archlinux-ax161> (raw)
In-Reply-To: <20211015094344.GQ8429@kadam>

On Fri, Oct 15, 2021 at 12:43:44PM +0300, Dan Carpenter wrote:
> On Thu, Oct 14, 2021 at 02:57:03PM -0700, Nathan Chancellor wrote:
> > A new warning in clang points out a place in this file where a bitwise
> > OR is being used with boolean expressions:
> > 
> > In file included from drivers/staging/wlan-ng/prism2usb.c:2:
> > drivers/staging/wlan-ng/hfa384x_usb.c:3787:7: warning: use of bitwise '|' with boolean operands [-Wbitwise-instead-of-logical]
> >             ((test_and_clear_bit(THROTTLE_RX, &hw->usb_flags) &&
> >             ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/staging/wlan-ng/hfa384x_usb.c:3787:7: note: cast one or both operands to int to silence this warning
> > 1 warning generated.
> 
> Both sides of this bitwise OR are bool, so | and || are equivalent
> logically.  Clang should not warn about it.

I do not disagree. The original motivation for the warning was code like

if (a() & b())

where a '&&' was intended to short circuit the call to b() if a() was
false but then it expanded to encompass bitwise OR as well. The clang
developers felt that warning on bitwise OR was worthwhile because most
of the time, '||' was intended. Feel free to comment on the Phabricator
thread if you feel strongly, there are not too many instances of this
warning and I think the '&' vs '&&' aspect of the warning is useful.

https://reviews.llvm.org/D108003

Cheers,
Nathan