Re: [PATCH 5/6] xfs: centralize error tag definitions

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Sep 15, 2025 at 10:53:27PM +0200, Christoph Hellwig wrote:
> On Mon, Sep 15, 2025 at 12:10:01PM -0700, Darrick J. Wong wrote:
> > > -#ifndef __XFS_ERRORTAG_H_
> > > +#if !defined(__XFS_ERRORTAG_H_) || defined(XFS_ERRTAG)
> > 
> > Hrmm.  So now xfs_errortag.h has two uses: one where we just #include
> > it without defining XFS_ERRTAG, and we get the XFS_ERRTAG_* values?
> 
> Yes.
> 
> > and I guess a broken XFS_ERRTAGS that isn't fully defined?
> 
> Yes.  I wouldn't call it broken, just not usable :)

Heh. :)

> > And then you're allowed to #include it again, provided you enclose that
> > inside a #define XFS_ERRTAG and a user of an XFS_ERRTAGS symbol, in
> > which case it actually spits out a usable XFS_ERRTAGS?
> 
> Yes.
> 
> > That could be documented better.
> > 
> > "There are two ways to use this header file.  The first way is to
> > #include it bare, which will define all the XFS_ERRTAG_* error injection
> > knobs for use with the XFS_TEST_ERROR macro.  The second way is to
> > enclose the #include with a #define for an XFS_ERRTAG macro, in which
> > case the header will define an XFS_ERRTAGS macro that expands to one
> > XFS_ERRTAG use for each defined error injection knob."
> 
> I can add that.
> 
> > > +
> > > +#define XFS_ERRTAGS \
> > 
> > Do you need to #undef XFS_ERRTAGS here to prevent the W=XXX build and/or
> > static tools from whining about redefined macros?
> 
> XFS_ERRTAGS is redefined identically, which is allowed.  And we can't
> undef it here, because we use the definition in the .c files.  We could
> undef it after using it, but that feels a bit pointless.

Sorry, I should have spelled out what I meant explicitly:

#ifdef XFS_ERRTAG
# undef XFS_ERRTAGS
# define XFS_ERRTAGS \
<giant blob of error knobs>
#endif /* XFS_ERRTAG */

You undefine XFS_ERRAGS *before* redefining it, just to make it clear to
anyone reading the .h file that the exact definition of XFS_ERRTAGS in a
.c file can be fluid.

--D

> > 
> > Also sorta wondering if for cleanliness this ought to be:
> > 
> > #ifdef XFS_ERRTAG
> > # define XFS_ERRTAGS \
> > <giant table>
> > #endif /* XFS_ERRTAG */
> > 
> > To avoid cluttering up the macro namespace if the file #include'ing
> > xfs_errortag.h didn't define XFS_ERRTAG and doesn't actually want the
> > ERRTAGS expansion?
> 
> That does sound doable, let me give it a try.
> 
>