Re: [RFC PATCH 00/11] IIO: Bring core header includes up to date.

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 31 Oct 2021 22:21:44 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sun, Oct 31, 2021 at 7:07 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > On Sun, 31 Oct 2021 17:08:54 +0000
> > Jonathan Cameron <jic23@kernel.org> wrote:  
> > > On Sun,  3 Oct 2021 16:32:55 +0100
> > > Jonathan Cameron <jic23@kernel.org> wrote:  
> 
> ...
> 
> > > Given ongoing churn in core kernel includes as Andy cleans them up, I've
> > > pushed this particular work out on a branch at
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=iio-iwyu-cleanups
> > >
> > > This will get me 0-day exposure and allow me to keep moving these
> > > forwards as the core kernel headers change.  
> 
> Thanks! I'm in favour of the work. Here are few things though:
> - blank line in between of the groups of headers (like before iio/*)

Ok. I was trying to leave style in keeping with what was there in drivers,
but fair enough can standardise on that.

> - fixing properly bitops.h and other typos (hope lkp will tell you about)

Huh. I'll watch out for that as getting clean builds...

> - thinking more about the list of very-low-level includes (see below)

Agreed.  I'm not sure on some of these - one of the big reasons this is a
work in progress.

> 
> On top of that, the main idea behind header usage is to make sure we
> have no circular and other hell dependencies in the *headers*, C-files
> can go with more relaxed rules (but again, I'm in favour of the series
> and effort).

Agreed, but added advantage of this tooling is identifying the entirely
pointless includes - of which there are lots.  Getting rid of those was
my main motivation in this.

Getting the balance right is going to be the trick and I'd rather a tool
that told us (more or less) that we have things correct than rely on 'common practice'.  

> 
> > > I would like to start merging 'some' of these in the meantime and
> > > there are some precursor cleanup patches that I'll pull out separately.
> > >
> > > Perhaps most 'useful' is the top patch in that branch which is the iwyu
> > > mapping file I've put together to get it to make more reasonable suggestions.
> > > Note that there are some cases where the answer isn't obvious and some
> > > where you can't push iwyu to do the right thing.
> > >
> > > One example is struct regmap which is deliberately opaque.
> > > iwyu always wants a forwards definition of it in all files where pointers
> > > to it occur, but personally I think including linux/regmap.h is sufficient
> > > as that will always have the forwards definition needed.
> > >
> > > Note this branch will rebase frequently and may well eat babies.
> > > In particularly I will be cherry picking Andy and anyone else's work
> > > to the top of it and making changes through the rest of the patches as
> > > that affects them.
> > >
> > > I'm not particularly expecting feedback, but I do want to avoid duplicate
> > > work.  I'm also likely to 'fixing' new code as it comes in based on this
> > > toolchain - I may main in reviews or just fix it whilst applying (and tell
> > > people obviously!)
> > >
> > > Long term plan here is to bring consistency to includes with benefits
> > > of resilience and hopefully reducing just how much code is actually pulled
> > > in whilst compiling.  
> >
> > I forgot to mention that I've take a stricter view since doing this series
> > so it needs an update even in the tree above. This mostly affects
> > err.h, errno.h, stddef.h and types.h  
> 
> I don't think we need to include stddef.h to every C file, it should
> be guaranteed via something like types.h or so.

There are places with stddef.h but not types.h required. I debated which
way to go on that and hence ended up with both for now, but it's obviously easy
to tweak.

It may be that long term we look to have a standard kernel iwyu mapping file.
It's possible to build in a fair bit of flexibility but maybe we'll need to
tweak the tool to allow us even more.

Jonathan


> 
> > I'll update those in that tree sometime this week.  
>