Re: [PATCH 5/9] surface3_power: avoid format string truncation warning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On Wed, Mar 27, 2024 at 1:05 AM Nathan Chancellor <nathan@xxxxxxxxxx> wrote:
> On Tue, Mar 26, 2024 at 11:38:04PM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@xxxxxxxx>
> >
> > clang warns about printing a pair of escaped strings into a buffer that is
> > too short:
> >
> > drivers/platform/surface/surface3_power.c:248:3: error: 'snprintf' will always be truncated; specified size is 10, but format string expands to at least 12 [-Werror,-Wformat-truncation-non-kprintf]
> >   248 |                 snprintf(bix->serial, ARRAY_SIZE(bix->serial), "%3pE%6pE", buf + 7, buf);
> >       |                 ^
> >
> > Change the format string two print two less bytes so it always fits. The string
> > is still truncated, so there is no change in behavior, but the compiler no
> > longer warns about it.

> > Fixes: 85f7582cd484 ("platform/surface: Move Surface 3 Power OpRegion driver to platform/surface")

Hmm... Is it really a move patch (which by title should not have
changed the contents)?
(I haven't looked into it, though.)

> > Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> > ---
> > Not entirely sure about this one, as I've never used escaped strings, and
> > don't know if gcc is correct to warn here, or if the kernel defines it
> > differently from the standard.
> As far as I understand it, this is a false positive because clang does
> not understand the kernel's %p extensions.

Yes, %pE here is special. Btw, what has already been discussed a long
time is to have a validation plugin in the kernel to check those %p
extensions, but IIUC nobody committed to it.

That said, the patch is most likely incorrect.

> GCC does not warn for
> overflow or truncation when %p is involved but the clang developers
> chose to intentionally deviate from GCC in that aspect while sticking it
> under a separate diagnostic that we could disable. I sent a patch that
> did so some time ago but I guess Masahiro never applied it...
> Consider dropping the changes that fix non-kprintf warnings and
> including that patch as part of this series.

With Best Regards,
Andy Shevchenko

[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux