[PATCH 0/3] common: Remove a few more header files

[Tom Rini]

On Mon, Sep 25, 2023 at 08:35:44AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 25 Sept 2023 at 08:08, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Sun, Sep 24, 2023 at 08:39:49PM -0600, Simon Glass wrote:
> >
> > > This series removes two more header files from the common.h header. It
> > > also tidies up code style for time.h functions, to simplify similar
> > > maintenance in future.
> > >
> > >
> > > Simon Glass (3):
> > >   Fix code style for time functions
> > >   common: Drop time.h from common header
> > >   common: Drop linux/string.h from common header
> > [snip]
> > >  1709 files changed, 1854 insertions(+), 34 deletions(-)
> >
> > This is where I start to get worried we're possibly taking the wrong
> > approach to the problem.  Perhaps it would be better to:
> > 1. Start finding headers which include <common.h>, remove and fixup the
> > breakage directly.
> > 2. Move on to removing <common.h> from files, a directory at a time.
> >
> 
> The bigger question is whether you want common.h gone or not?

Yes, I'd like it gone.

> I shared my script with you so you can see what it is doing. But if a
> board uses memcmp(), for example, then it needs linux/string.h
> included. We can discuss why so many files use string functions, or
> whether the script is not perfect, but ultimately that is the
> question.
> 
> Just these simple queries give you a sense of the scale:
> 
>  git grep -l strcat |wc
>      45      45    1134
> git grep -l memcpy |wc
>     834     834   21643
> 
> If you are suggesting that we add fewer includes for now, relying on
> transitive includes in header files, then I don't think that is very
> useful in the end. I had a series a few years back which removed
> common.h entirely and it foundered on this same issue.

The problem I see with the approach you're taking now is that it's quite
hard to provide meaningful review.  Looking at the time.h patch here, I
assume you agree it would be better to just remove 
arch/arm/include/asm/arch-tegra/timer.h as it's just a single prototype
already found in include/time.h.  And the example where time.h was
included inside a comment block is a tooling issue.  But then looking at
the printk.h patch I did just apply, I had to drop a few files because
they introduced seemingly random changes to the resulting build on some
platforms.  And something like that is much easier to deal with and
explain _why_ it's happening if we're removing the header file by file,
rather than world-wide move-a-header.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230925/9e3b6145/attachment.sig>