Re: [PATCH v2 2/3] lib/vsprintf: Split out sprintf() and friends

[Petr Mladek <pmladek@suse.com>]

On Tue 2023-08-08 15:47:59, Andy Shevchenko wrote:
> On Tue, Aug 08, 2023 at 08:41:49AM +0200, Petr Mladek wrote:
> > On Mon 2023-08-07 18:13:57, Andy Shevchenko wrote:
> > > On Mon, Aug 07, 2023 at 06:11:24PM +0300, Andy Shevchenko wrote:
> > > > On Mon, Aug 07, 2023 at 06:09:54PM +0300, Andy Shevchenko wrote:
> > > > > On Mon, Aug 07, 2023 at 05:03:19PM +0200, Petr Mladek wrote:
> > > > > > On Sat 2023-08-05 20:50:26, Andy Shevchenko wrote:
> 
> ...
> 
> > > > > > How does this sound, please?
> > > > > 
> > > > > Not every user (especially _header_) wants to have printk.h included just for
> > > > > sprintf.h that may have nothing to do with real output. So, same reasoning
> > > > > from me as keeping that in kernel.h, i.e. printk.h no better.
> > > > 
> > > > (haven't check these, just to show how many _headers_ uses sprintf() call)
> > > > 
> > > > $ git grep -lw s.*printf -- include/linux/
> > > > include/linux/acpi.h
> > > > include/linux/audit.h
> > > > include/linux/btf.h
> > > > include/linux/dev_printk.h
> > > > include/linux/device-mapper.h
> > > > include/linux/efi.h
> > > > include/linux/fortify-string.h
> > > > include/linux/fs.h
> > > > include/linux/gameport.h
> > > > include/linux/kdb.h
> > > > include/linux/kdev_t.h
> > > > include/linux/kernel.h
> > > > include/linux/mmiotrace.h
> > > > include/linux/netlink.h
> > > > include/linux/pci-p2pdma.h
> > > > include/linux/perf_event.h
> > > > include/linux/printk.h
> > > > include/linux/seq_buf.h
> > > > include/linux/seq_file.h
> > > > include/linux/shrinker.h
> > > > include/linux/string.h
> > > > include/linux/sunrpc/svc_xprt.h
> > > > include/linux/tnum.h
> > > > include/linux/trace_seq.h
> > > > include/linux/usb.h
> > > > include/linux/usb/gadget_configfs.h
> > > 
> > > Okay, revised as my regexp was too lazy
> > > 
> > > $ git grep -lw s[^[:space:]_]*printf -- include/linux/
> > > include/linux/btf.h
> > > include/linux/device-mapper.h
> > > include/linux/efi.h
> > > include/linux/fortify-string.h
> > > include/linux/kdev_t.h
> > > include/linux/kernel.h
> > > include/linux/netlink.h
> > > include/linux/pci-p2pdma.h
> > > include/linux/perf_event.h
> > > include/linux/sunrpc/svc_xprt.h
> > > include/linux/tnum.h
> > > include/linux/usb.h
> > > include/linux/usb/gadget_configfs.h
> > 
> > This is only a tiny part of the picture.
> > 
> > $> git grep sc*n*printf | cut -d : -f1 | uniq | grep "\.c$" | wc -l
> > 5254
> > $> find . -name  "*.c" | wc -l
> > 32319
> > 
> > It means that the vsprintf() family is used in 1/6 of all kernel
> > source files. They would need to include one extra header.
> 
> No, not only one. more, but the outcome of this is not using what is not used
> and unwinding the header dependency hell.
> 
> But hey, I am not talking about C files right now, it's secondary, however
> in IIO we want to get rid of kernel.h in the C files as well.

This sounds scary. Headers and C files are closely related. IMHO, it
does not makes sense to split header files without looking how
the functions are used.

Everyone agrees that kernel.h should be removed. But there are always
more possibilities where to move the definitions. For this, the use
in C files must be considered. Otherwise, it is just a try&hope approach.

> Also, please, go through all of them and tell, how many of them are using
> stuff from kernel.h besides sprintf.h and ARRAY_SIZE() (which I plan
> for a long time to split from kernel.h)?

I am all for removing vsprintf declarations from linux.h.

I provided the above numbers to support the idea of moving them
into printk.h.

The numbers show that the vsprintf function famility is used
quite frequently. IMHO, creating an extra tiny include file
will create more harm then good. By the harm I mean:

    + churn when updating 1/6 of source files

    + prolonging the list of #include lines in .c file. It will
      not help with maintainability which was one of the motivation
      in this patchset.

    + an extra work for people using vsprintf function family in
      new .c files. People are used to get them for free,
      together with printk().

Best Regards,
Petr