On Thu, Jan 30, 2020 at 10:59:22AM -0600, Eric Sandeen wrote: > On 6/20/19 5:06 PM, Dave Chinner wrote: > > On Thu, Jun 20, 2019 at 04:29:23PM -0500, Eric Sandeen wrote: > >> This is the result of a mechanical process and ... may have a few > >> oddities, for example removing "init.h" from some utils made me > >> realize that we inherit it from libxfs and also have it in local > > > > We do? That'd be really, really broken if we did - including local > > header files from a global header files is not a good idea. > > > > /me goes looking, can't find where libxfs.h includes init.h > > > > libxfs/init.h is private to libxfs/, it's not a global include file, > > and it is included directly in all the libxfs/*.c files that need > > it, which is 3 files - init.c, rdwr.c and util.c. > > > >> headers; libxfs has a global but so does scrub, etc. So that stuff > >> can/should be fixed up, but in the meantime, this zaps out a ton > >> of header dependencies, and seems worthwhile. > > > > IMO, this doesn't improve the tangle of header files in userspace. > > All it does is make the include patterns inconsistent across files > > because of the tangled mess of the libxfs/ vs include/ header files > > that was never completely resolved when libxfs was created as a > > shared kernel library.... > > > > IOWs, the include pattern I was originally aiming for with the > > libxfs/ shared userspace/kernel library was: > > > > #include "libxfs_priv.h" > > <include shared kernel header files> > > > > And for things outside libxfs/ that use libxfs: > > > > #include "libxfs.h" > > <include local header files> > > > > IOWs, "libxfs_priv.h" contained the includes for all the local > > userspace libxfs includes and defines and non-shared support > > structures, and it would export on build all the header files that > > external code needs to build into include/ via symlinks. This is > > incomplete - stuff like include/xfs_mount.h, xfs_inode.h, etc needs > > to move into libxfs as private header files (similar to how they are > > private in the kernel) and then exported at build time. > > > > Likewise, "libxfs.h" should only contain global include files and > > those exported from libxfs, and that's all the external code should > > include to use /anything/ from libxfs. i.e. a single include forms > > the external interface to libxfs. > > > > AFAIC, nothing should be including platform or build dependent > > things like platform_defs.h, because that should be pulled in by > > libxfs.h or libxfs_priv.h. And nothing external should need to pull > > in, say, xfs_format.h or xfs_mount.h, because they are all pulled in > > by include/libxfs.h (which it mostly does already). > > > > Hence I'd prefer we finish untangling the header file include mess > > before we cull unneceesary includes. Otherwise we are going to end > > up culling the wrong includes and then have to clean up that mess as > > well to bring the code back to being clean and consistent.... > > Dave, what if I reworked this to only remove unneeded system include files > for now. Would you be more comfortable with that? I assume there's no > argument with i.e. > > diff --git a/estimate/xfs_estimate.c b/estimate/xfs_estimate.c > index 9e01cce..189bb6c 100644 > --- a/estimate/xfs_estimate.c > +++ b/estimate/xfs_estimate.c > @@ -10,7 +10,6 @@ > * XXX: assumes dirv1 format. > */ > #include "libxfs.h" > -#include <sys/stat.h> > #include <ftw.h> > > right? *nod* That sort of thing can easily be cleaned up. -Dave. -- Dave Chinner david@xxxxxxxxxxxxx
