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 <tr...@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
signature.asc
Description: PGP signature