From: Dave Chinner <david@fromorbit.com>
To: Eric Sandeen <sandeen@redhat.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 00/11] xfsprogs: remove unneeded #includes
Date: Fri, 21 Jun 2019 08:06:14 +1000 [thread overview]
Message-ID: <20190620220614.GE26375@dread.disaster.area> (raw)
In-Reply-To: <1561066174-13144-1-git-send-email-sandeen@redhat.com>
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....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2019-06-20 22:07 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-20 21:29 [PATCH 00/11] xfsprogs: remove unneeded #includes Eric Sandeen
2019-06-20 21:29 ` [PATCH 01/11] xfs_estimate: remove unneeded includes Eric Sandeen
2019-06-25 11:01 ` Christoph Hellwig
2019-06-20 21:29 ` [PATCH 02/11] xfs_fsr: " Eric Sandeen
2019-06-20 21:29 ` [PATCH 03/11] xfs_io: " Eric Sandeen
2019-06-20 21:29 ` [PATCH 04/11] libfrog: " Eric Sandeen
2019-06-20 21:29 ` [PATCH 05/11] libxcmd: " Eric Sandeen
2019-06-20 21:29 ` [PATCH 06/11] libxfs: " Eric Sandeen
2019-06-20 21:29 ` [PATCH 07/11] xfs_logprint: " Eric Sandeen
2019-06-20 21:29 ` [PATCH 08/11] xfs_quota: " Eric Sandeen
2019-06-20 21:29 ` [PATCH 09/11] xfs_repair: " Eric Sandeen
2019-06-20 21:29 ` [PATCH 10/11] xfs_scrub: " Eric Sandeen
2019-06-20 21:29 ` [PATCH 11/11] xfs_spaceman: " Eric Sandeen
2019-06-20 22:06 ` Dave Chinner [this message]
2020-01-30 16:59 ` [PATCH 00/11] xfsprogs: remove unneeded #includes Eric Sandeen
2020-01-31 2:11 ` Dave Chinner
2019-06-25 11:01 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190620220614.GE26375@dread.disaster.area \
--to=david@fromorbit.com \
--cc=linux-xfs@vger.kernel.org \
--cc=sandeen@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).