From: Hans de Goede <hdegoede@redhat.com>
To: Arnd Bergmann <arnd@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Dan Carpenter <dan.carpenter@oracle.com>,
Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
Sparse Mailing-list <linux-sparse@vger.kernel.org>,
Nathan Chancellor <nathan@kernel.org>,
Nick Desaulniers <ndesaulniers@google.com>,
Al Viro <viro@zeniv.linux.org.uk>, Arnd Bergmann <arnd@arndb.de>,
Linux FS-devel Mailing List <linux-fsdevel@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
llvm@lists.linux.dev
Subject: Re: [PATCH] vboxsf: fix old signature detection
Date: Tue, 28 Sep 2021 12:31:03 +0200 [thread overview]
Message-ID: <42797736-a64b-e244-136a-d4526b732a50@redhat.com> (raw)
In-Reply-To: <CAK8P3a3sEy7NAhMHcV7XPpZxo5tHnQz1oCP43YTe_ZQuzOHgPA@mail.gmail.com>
Hi,
On 9/28/21 12:11 PM, Arnd Bergmann wrote:
> On Tue, Sep 28, 2021 at 11:40 AM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 9/27/21 8:33 PM, Linus Torvalds wrote:
>>> On Mon, Sep 27, 2021 at 6:22 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>>>
>>>> More specifically, ' think '\377' may be either -1 or 255 depending on
>>>> the architecture.
>>>> On most architectures, 'char' is implicitly signed, but on some others
>>>> it is not.
>>>
>>> Yeah. That code is just broken.
>>>
>>> And Arnd, your patch may be "conceptually minimal", in that it keeps
>>> thed broken code and makes it work. But it just dials up the oddity to
>>> 11.
>
> Thank you for addressing it. I usually try to avoid overthinking changes
> to "unusual" code like this, but your solution is clearly an improvement.
>
> What really threw me off this time is that my first attempt to address
> the warning was an exact revert of 9d682ea6bcc7 ("vboxsf: Fix the
> check for the old binary mount-arguments struct"), which in turn
> came from a tool that is usually correct and and that both Dan
> and Al thought the original patch was correct when it looked like
> it turned a working (though unusual) implementation into a broken
> one.
>
>> I agree that your suggestion is to be the best solution,
>> so how do we move forward with this, do I turn this into a
>> proper patch with you as the author and Arnd as Reported-by and
>> if yes may I add your Signed-off-by to the patch ?
>
> It's already upstream, see d5f6545934c4 ("qnx4: work around gcc
> false positive warning bug").
Ah, actually you mean: 9b3b353ef330 ("vboxfs: fix broken legacy mount
signature checking"), but other then that yes you're right it
is already upstream.
Thank you Arnd and thank you Linus.
Regards,
Hans
next prev parent reply other threads:[~2021-09-28 10:31 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-27 9:40 [PATCH] vboxsf: fix old signature detection Arnd Bergmann
2021-09-27 10:09 ` Hans de Goede
2021-09-27 13:02 ` Dan Carpenter
2021-09-27 13:21 ` Arnd Bergmann
2021-09-27 18:33 ` Linus Torvalds
2021-09-28 9:39 ` Hans de Goede
2021-09-28 10:11 ` Arnd Bergmann
2021-09-28 10:31 ` Hans de Goede [this message]
2021-09-28 10:40 ` Arnd Bergmann
2021-09-28 9:56 ` Rasmus Villemoes
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=42797736-a64b-e244-136a-d4526b732a50@redhat.com \
--to=hdegoede@redhat.com \
--cc=arnd@arndb.de \
--cc=arnd@kernel.org \
--cc=dan.carpenter@oracle.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sparse@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=luc.vanoostenryck@gmail.com \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).