On Tue, May 06, 2014 at 01:37:44AM -0700, Christoph Hellwig wrote:
> I like this in general, but one major and one minor issue with
> the include files:
> - headers that just include other headers are a bad idea in general.
> Either they are dependent enough that they should be merged, or
> they are not, in which case they shouldn't.
> In this case it seems like we should temporarily provide a
> xfs_mount.h stub in userspace, and just leave all the includes
> for things in libxfs.h as they were. That doesn't preclude further
> merging of the headers into more sensible ones as we've started
> with the disk formats.
I did this because I'm sick of having to edit 50+ files whenever a
single header dependency changes. There are almost all cookie cutter
duplicates because of the dependencies - if it were code, we'd
factor it in an instant. I just don't see why we should treat 50
copies of the same header includes any differently....
> - do we really need the separate include/ dir? That always annoys
> me when editing code. It makes sense for something that is a real
> public interface, which this is not.
It's for simplicity of updates with the userspace code. Both
userspace and kernel need the same code layout, and userspace
currently has a separate include directory for all the header files
(and they get installed that way, too). If we want to change the
userspace source layout and commingle all the headers with the C
code, then that's a lot more work on the userspace side (i.e. it's
more than just pointing the include/xfs symlink to libxfs/include).
I don't mind which approach we take - it's trivial to rework the
patchset to place all the headers in the libxfs/ directory - I just
took the one that matched the current userspace infrastructure...
> Also is libxfs/ really the right name? libxfs in userspace has quite
> a bit more code than this, so maybe we should just called this "shared"
> for the shared user/kernel code?
I'd like to have this kernel code define libxfs/, while in userspace
we separate out all the support code (i.e. libxfs/rdwr.c, etc) into
a different directory that builds the userspace libraries. i.e.
libxfs/ is a static object archive that is wrapped by the userspace
infrastructure, just like the kernel wraps it with infrastructure to
make it useful...