On Tue, Sep 28, 2021 at 11:40 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > On 9/27/21 8:33 PM, Linus Torvalds wrote: > > On Mon, Sep 27, 2021 at 6:22 AM Arnd Bergmann <arnd@xxxxxxxxxx> 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"). Arnd