mbox series

[v2,0/6] vsprintf: Add __printf attribute to where it's required

Message ID 20250321144822.324050-1-andriy.shevchenko@linux.intel.com (mailing list archive)
Headers show
Series vsprintf: Add __printf attribute to where it's required | expand

Message

Andy Shevchenko March 21, 2025, 2:40 p.m. UTC
This whole series started from a simple fix (see the last patch)
to make GCC (Debian 14.2.0-17) happy when compiling with `make W=1`
(note, that CONFIG_WERROR=y and all warnings break the build!)
down to a rabbit hole.

However starting from v2 the last patch doesn't require the first
part, I prefer still to have them since the functions, while being
_binary_ printf()-like, are still printf()-like. It also puts in align
the tracing stuff with the rest and fixes the wrong parameter value.

These first 4 patches are organised in a strict order and can't be
reshuffled, otherwise it will produce a warnings in between.

I believe the best route for the series is printk tree with immutable
tag or branch for the others.

Alternatively the first 4 patches can be applied first as they
are pretty much straightforward. They also can be squashed to one
(as the same topic behind), but it all is up to the respective
maintainers.

In v2:
- split out patch 5 (Rasmus)
- rewritten the approach for the va_format() fix (Rasmus)
- amended tracing patch (removed a blank line and a __printf() in C file)

Andy Shevchenko (6):
  seq_buf: Mark binary printing functions with __printf() attribute
  seq_file: Mark binary printing functions with __printf() attribute
  tracing: Mark binary printing functions with __printf() attribute
  vsnprintf: Mark binary printing functions with __printf() attribute
  vsnprintf: Drop unused const char fmt * in va_format()
  vsnprintf: Silence false positive GCC warning for va_format()

 include/linux/seq_buf.h   |  4 ++--
 include/linux/seq_file.h  |  1 +
 include/linux/string.h    |  4 ++--
 include/linux/trace.h     |  4 ++--
 include/linux/trace_seq.h |  8 ++++----
 kernel/trace/trace.c      | 11 +++--------
 kernel/trace/trace.h      | 16 +++++++++-------
 lib/vsprintf.c            |  9 +++++++--
 8 files changed, 30 insertions(+), 27 deletions(-)

Comments

Petr Mladek March 25, 2025, 10:15 a.m. UTC | #1
On Fri 2025-03-21 16:40:46, Andy Shevchenko wrote:
> This whole series started from a simple fix (see the last patch)
> to make GCC (Debian 14.2.0-17) happy when compiling with `make W=1`
> (note, that CONFIG_WERROR=y and all warnings break the build!)
> down to a rabbit hole.
> 
> However starting from v2 the last patch doesn't require the first
> part, I prefer still to have them since the functions, while being
> _binary_ printf()-like, are still printf()-like. It also puts in align
> the tracing stuff with the rest and fixes the wrong parameter value.
> 
> These first 4 patches are organised in a strict order and can't be
> reshuffled, otherwise it will produce a warnings in between.
> 
> I believe the best route for the series is printk tree with immutable
> tag or branch for the others.
> 
> Alternatively the first 4 patches can be applied first as they
> are pretty much straightforward. They also can be squashed to one
> (as the same topic behind), but it all is up to the respective
> maintainers.

The whole series looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

I am going to push it via the printk tree. I think about doing
so as a second pull request by the end of this merge window.

Anyway, I am going to wait few more days for eventual feedback
or push back.

Best Regards,
Petr
Kees Cook March 25, 2025, 7:38 p.m. UTC | #2
On Fri, Mar 21, 2025 at 04:40:46PM +0200, Andy Shevchenko wrote:
> This whole series started from a simple fix (see the last patch)
> to make GCC (Debian 14.2.0-17) happy when compiling with `make W=1`
> (note, that CONFIG_WERROR=y and all warnings break the build!)
> down to a rabbit hole.
> 
> However starting from v2 the last patch doesn't require the first
> part, I prefer still to have them since the functions, while being
> _binary_ printf()-like, are still printf()-like. It also puts in align
> the tracing stuff with the rest and fixes the wrong parameter value.
> 
> These first 4 patches are organised in a strict order and can't be
> reshuffled, otherwise it will produce a warnings in between.
> 
> I believe the best route for the series is printk tree with immutable
> tag or branch for the others.
> 
> Alternatively the first 4 patches can be applied first as they
> are pretty much straightforward. They also can be squashed to one
> (as the same topic behind), but it all is up to the respective
> maintainers.
> 
> In v2:
> - split out patch 5 (Rasmus)
> - rewritten the approach for the va_format() fix (Rasmus)
> - amended tracing patch (removed a blank line and a __printf() in C file)
> 
> Andy Shevchenko (6):
>   seq_buf: Mark binary printing functions with __printf() attribute
>   seq_file: Mark binary printing functions with __printf() attribute
>   tracing: Mark binary printing functions with __printf() attribute
>   vsnprintf: Mark binary printing functions with __printf() attribute
>   vsnprintf: Drop unused const char fmt * in va_format()
>   vsnprintf: Silence false positive GCC warning for va_format()
> 
>  include/linux/seq_buf.h   |  4 ++--
>  include/linux/seq_file.h  |  1 +
>  include/linux/string.h    |  4 ++--
>  include/linux/trace.h     |  4 ++--
>  include/linux/trace_seq.h |  8 ++++----
>  kernel/trace/trace.c      | 11 +++--------
>  kernel/trace/trace.h      | 16 +++++++++-------
>  lib/vsprintf.c            |  9 +++++++--
>  8 files changed, 30 insertions(+), 27 deletions(-)

Cool; it'll be nice to get these marked up.

Reviewed-by: Kees Cook <kees@kernel.org>
Petr Mladek March 28, 2025, 1:51 p.m. UTC | #3
On Tue 2025-03-25 11:15:21, Petr Mladek wrote:
> On Fri 2025-03-21 16:40:46, Andy Shevchenko wrote:
> > This whole series started from a simple fix (see the last patch)
> > to make GCC (Debian 14.2.0-17) happy when compiling with `make W=1`
> > (note, that CONFIG_WERROR=y and all warnings break the build!)
> > down to a rabbit hole.
> > 
> > However starting from v2 the last patch doesn't require the first
> > part, I prefer still to have them since the functions, while being
> > _binary_ printf()-like, are still printf()-like. It also puts in align
> > the tracing stuff with the rest and fixes the wrong parameter value.
> > 
> > These first 4 patches are organised in a strict order and can't be
> > reshuffled, otherwise it will produce a warnings in between.
> > 
> > I believe the best route for the series is printk tree with immutable
> > tag or branch for the others.
> > 
> > Alternatively the first 4 patches can be applied first as they
> > are pretty much straightforward. They also can be squashed to one
> > (as the same topic behind), but it all is up to the respective
> > maintainers.
> 
> The whole series looks good to me:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> 
> I am going to push it via the printk tree. I think about doing
> so as a second pull request by the end of this merge window.
> 
> Anyway, I am going to wait few more days for eventual feedback
> or push back.

JFYI, I have pushed the patchset into printk/linux.git,
branch for-6.15-printf-attribute.

I am going to send a pull request the following week
if nothing happens in the meantime.

Best Regards,
Petr
Andy Shevchenko March 28, 2025, 3:05 p.m. UTC | #4
On Fri, Mar 28, 2025 at 3:51 PM Petr Mladek <pmladek@suse.com> wrote:
> On Tue 2025-03-25 11:15:21, Petr Mladek wrote:
> > On Fri 2025-03-21 16:40:46, Andy Shevchenko wrote:
> > > This whole series started from a simple fix (see the last patch)
> > > to make GCC (Debian 14.2.0-17) happy when compiling with `make W=1`
> > > (note, that CONFIG_WERROR=y and all warnings break the build!)
> > > down to a rabbit hole.
> > >
> > > However starting from v2 the last patch doesn't require the first
> > > part, I prefer still to have them since the functions, while being
> > > _binary_ printf()-like, are still printf()-like. It also puts in align
> > > the tracing stuff with the rest and fixes the wrong parameter value.
> > >
> > > These first 4 patches are organised in a strict order and can't be
> > > reshuffled, otherwise it will produce a warnings in between.
> > >
> > > I believe the best route for the series is printk tree with immutable
> > > tag or branch for the others.
> > >
> > > Alternatively the first 4 patches can be applied first as they
> > > are pretty much straightforward. They also can be squashed to one
> > > (as the same topic behind), but it all is up to the respective
> > > maintainers.
> >
> > The whole series looks good to me:
> >
> > Reviewed-by: Petr Mladek <pmladek@suse.com>
> >
> > I am going to push it via the printk tree. I think about doing
> > so as a second pull request by the end of this merge window.
> >
> > Anyway, I am going to wait few more days for eventual feedback
> > or push back.
>
> JFYI, I have pushed the patchset into printk/linux.git,
> branch for-6.15-printf-attribute.
>
> I am going to send a pull request the following week
> if nothing happens in the meantime.

Thank you!
It will make us much closer to the moment when we can enable WERROR=y
in the CIs for `make W=1` on x86 defconfigs (those that are in the
kernel source tree).