Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20211018182537.2316800-1-nathan@kernel.org> <CAKwvOdn4_DrgaZoAo-v7CbYurNUpfAK5tnzT023=WCDzkmYQVg@mail.gmail.com>
In-Reply-To: <CAKwvOdn4_DrgaZoAo-v7CbYurNUpfAK5tnzT023=WCDzkmYQVg@mail.gmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Mon, 18 Oct 2021 09:41:15 -1000
Message-ID: <CAHk-=wi7hUsTTcmPfZCkUEw51Y3ayq3JJxzFsNgodsxxDyk9Ww@mail.gmail.com>
Subject: Re: [PATCH] platform/x86: thinkpad_acpi: Fix bitwise vs. logical warning
To:     Nick Desaulniers <ndesaulniers@google.com>
Cc:     Nathan Chancellor <nathan@kernel.org>,
        Henrique de Moraes Holschuh <hmh@hmh.eng.br>,
        Hans de Goede <hdegoede@redhat.com>,
        Mark Gross <markgross@kernel.org>,
        ibm-acpi-devel@lists.sourceforge.net,
        platform-driver-x86@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        llvm@lists.linux.dev, Tor Vic <torvic9@mailbox.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Oct 18, 2021 at 8:34 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> LGTM, thanks for the patch! I guess this would be the first
> "interesting" case this warning has found in kernel sources?

The patch looks obviously correct (tm), but I'm not convinced that the
warning is actually all that interesting.

The thing is, using bitwise operators for booleans is _exactly_ the
same as using logical ones as long as there are no side effects. In
fact, any compiler worth its salt will already convert some cases
between the two as an optimization just as part of code generation.

Of course, that "as long as there are no side effects" is the big
thing - then the short-circuiting of the actual logical operations
clearly matters. But that wasn't actually the case in this situation
(or in the kvm situation elsewhere).

So in both of these cases, the difference between "|" and "||" ends up
purely being a hint to the compiler.

In this case, even if there are no side effects, it's clearly
pointless to do the second strlencmp() if the first one already
matched, and the "||" is unquestionably the right hint (and honestly,
most compilers probably wouldn't even be able to tell "no side
effects" because it's a fairly complex expression - but since it's
inlined and uses compiler intrinsics, the compiler _might_ actually be
able to see that the two are equivalent).

But no, I don't think that warning is very interesting. In fact, the
warning might be overall detrimental, in case the hints were
intentional (like the kvm case - although I'm not convinced the kvm
hinting was actually meaningful).

                 Linus