Re: [PATCH 1/2] tracing: work around -Wmissing-format-attribute warning
Rasmus Villemoes <[email protected]> Wed, 03 Jun 2026 09:15:11 +0200
| Newsgroups | dev.linux.lists.brcm80211,dev.linux.lists.llvm,org.kernel.vger.linux-kernel,org.kernel.vger.linux-rdma,org.kernel.vger.linux-trace-kernel,org.kernel.vger.linux-wireless |
|---|---|
| Message-ID | <[email protected]> |
On Tue, Jun 02 2026, "Arnd Bergmann" <[email protected]> wrote: > On Tue, Jun 2, 2026, at 20:59, Andy Shevchenko wrote: >> On Tue, Jun 02, 2026 at 05:07:05PM +0200, Arnd Bergmann wrote: >>> >>> A number of tracing headers turn off -Wsuggest-attribute=format for >>> gcc, but they don't turn it off for clang, so the same warning still >>> happens on new versions of clang that support the format attribute. >>> >>> To avoid duplicating the same thing in each tracing header, as well >>> as changing all of them to also turn it off for clang, add a new >>> __vsnprintf() helper that is not annotated this way in linux/sprintf.h >>> but is defined to work the same way as the regular vsprintf. >> >> vsprintf() > > Fixed now > >> Why the __printf() annotation is in the C file and not here? >> Is this all about headers as the second paragraph in the commit message >> explains? >> I would add a comment to explain it here, otherwise we might see false >> patches to "make things consistent" in a wrong way. > > I've tried to come up with a kerneldoc comment now, similar to > the one for the vsnprintf() function, and added a separate prototype > in the header. Does this address your concern? > > Arnd > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > index 3caf0796f54d..7c696aea2ed3 100644 > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -2975,7 +2975,23 @@ int vsnprintf(char *buf, size_t size, const char *fmt_str, va_list args) > } > EXPORT_SYMBOL(vsnprintf); > > -int __printf(3, 0) __vsnprintf(char *buf, size_t size, const char *fmt_str, va_list args) > +/** > + * __vsnprintf - vsnprintf() wrapper without __printf() attribute > + * @buf: The buffer to place the result into > + * @size: The size of the buffer, including the trailing null space > + * @fmt_str: The format string to use > + * @args: Arguments for the format string > + * > + * This has the exact same behavior as vsnprintf() but can be used in call > + * sites that are missing a __printf() annotation, e.g. because they > + * get a 'va_format' argument instead of format and varargs. > + * > + * For this to work, the attribute is added to the declaration here but > + * not in the header. > + */ > +int __printf(3, 0) __vsnprintf(char *buf, size_t size, const char *fmt_str, va_list args); > + > +int __vsnprintf(char *buf, size_t size, const char *fmt_str, va_list args) > { > return vsnprintf(buf, size, fmt_str, args); > } May I suggest a different approach, that avoids having that extra function emitted (which presumably compiles to a single jump instruction, but still, with retpoline and CFI and all that it all adds up): Keep the declaration of __vsnprintf() in the header without the __print() attribute, but then do int __vsnprintf(char *buf, size_t size, const char *fmt_str, va_list args) __alias(vsnprintf); in vsprintf.c. Aside from reusing the same entry point, I could well imagine a compiler some day complaining about seeing the printf attribute applied in a local extra declaration but not having it in the header file. Presumably it will need its own EXPORT_SYMBOL if any of the intended users are modular, and it certainly still needs a comment. Rasmus