Re: [PATCH 02/12] libxfs: move topology declarations into separate header

Darrick J. Wong
2019-09-03T03:16:07Z

On Tue, Sep 03, 2019 at 08:33:46AM +1000, Dave Chinner wrote:
> On Fri, Aug 30, 2019 at 01:34:02PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > >
> > > > The topology functions live in libfrog now, which means their
> > > > declarations don't belong in libxcmd.h. Create new header file for
> > > > them.
> > > >
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > ---
> > > > include/libxcmd.h | 31 -------------------------------
> > > > include/topology.h | 39 +++++++++++++++++++++++++++++++++++++++
> > > > libfrog/topology.c | 1 +
> > > > mkfs/xfs_mkfs.c | 2 +-
> > > > repair/sb.c | 1 +
> > > > 5 files changed, 42 insertions(+), 32 deletions(-)
> > > > create mode 100644 include/topology.h
> > >
> > > I like the idea, but I'm wondering if we should have a similar
> > > setup to libxfs header files here.
> > >
> > > i.e. the header file lives in libfrog/, and the install-headers make
> > > command creates include/libxfrog and links them for the build. That
> > > way the includes become namespaced like:
> > >
> > > #include "libxfrog/topology,h"
> > >
> > > and we don't pollute include with random header files from all
> > > different parts of xfsprogs...
> >
> > What if I leave topology.h in libfrog/ and then create an
> > include/libfrog.h that pulls in all the libfrog header files like
> > libxfs.h does, and then put -I$(TOPDIR)/libfrog in GCFLAGS?
>
> libxfs is a basket case - it's done that way so we don't need all
> the whacky games we play to shim the kernel functionality correctly
> in every xfsprogs file that needs libxfs functionality.
>
> libxfrog is very different - we have progs that just need topology
> or number conversion, but not both. I'd prefer for libxfrog we only
> include the headers we require, that way avoiding defining things we
> don't actually need...

Ok, will rework this tomorrow.

--D

> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com