Re: [PATCH 2/4] repair: add canonical names for the XR_INO_ constants

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

On Fri, Nov 28, 2025 at 07:37:00AM +0100, Christoph Hellwig wrote:
> Add an array with the canonical name for each inode type so that code
> doesn't have to implement switch statements for that, and remove the now
> trivial process_misc_ino_types and process_misc_ino_types_blocks
> functions.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  repair/dinode.c | 157 +++++++++++++++---------------------------------
>  1 file changed, 48 insertions(+), 109 deletions(-)
> 
> diff --git a/repair/dinode.c b/repair/dinode.c
> index b824dfc0a59f..fd40fdcce665 100644
> --- a/repair/dinode.c
> +++ b/repair/dinode.c
> @@ -43,6 +43,25 @@ enum xr_ino_type {
>  	XR_INO_RTREFC,		/* realtime refcount */
>  };
>  
> +static const char *xr_ino_type_name[] = {
> +	[XR_INO_UNKNOWN]	= "unknown",
> +	[XR_INO_DIR]		= "directory",
> +	[XR_INO_RTDATA]		= "realtime file",
> +	[XR_INO_RTBITMAP]	= "realtime bitmap",
> +	[XR_INO_RTSUM]		= "realtime summary",
> +	[XR_INO_DATA]		= "regular file",
> +	[XR_INO_SYMLINK]	= "symlink",
> +	[XR_INO_CHRDEV]		= "character device",
> +	[XR_INO_BLKDEV]		= "block device",
> +	[XR_INO_SOCK]		= "socket",
> +	[XR_INO_FIFO]		= "fifo",
> +	[XR_INO_UQUOTA]		= "user quota",
> +	[XR_INO_GQUOTA]		= "group quota",
> +	[XR_INO_PQUOTA]		= "project quota",
> +	[XR_INO_RTRMAP]		= "realtime rmap",
> +	[XR_INO_RTREFC]		= "realtime refcount",
> +};

Might want a

BUILD_BUG_ON(ARRAY_SIZED(xr_ino_type_name) != XR_INO_MAX);

so that someone adding to the enum won't forget to add a string here.

> +
>  /*
>   * gettext lookups for translations of strings use mutexes internally to
>   * the library. Hence when we come through here doing parallel scans in
> @@ -1946,106 +1965,6 @@ _("found illegal null character in symlink inode %" PRIu64 "\n"),
>  	return(0);
>  }
>  
> -/*
> - * called to process the set of misc inode special inode types
> - * that have no associated data storage (fifos, pipes, devices, etc.).
> - */
> -static int
> -process_misc_ino_types(
> -	xfs_mount_t		*mp,
> -	struct xfs_dinode	*dino,
> -	xfs_ino_t		lino,
> -	enum xr_ino_type	type)
> -{
> -	/*
> -	 * must also have a zero size
> -	 */
> -	if (be64_to_cpu(dino->di_size) != 0)  {
> -		switch (type)  {
> -		case XR_INO_CHRDEV:
> -			do_warn(
> -_("size of character device inode %" PRIu64 " != 0 (%" PRId64 " bytes)\n"), lino,
> -				(int64_t)be64_to_cpu(dino->di_size));
> -			break;
> -		case XR_INO_BLKDEV:
> -			do_warn(
> -_("size of block device inode %" PRIu64 " != 0 (%" PRId64 " bytes)\n"), lino,
> -				(int64_t)be64_to_cpu(dino->di_size));
> -			break;
> -		case XR_INO_SOCK:
> -			do_warn(
> -_("size of socket inode %" PRIu64 " != 0 (%" PRId64 " bytes)\n"), lino,
> -				(int64_t)be64_to_cpu(dino->di_size));
> -			break;
> -		case XR_INO_FIFO:
> -			do_warn(
> -_("size of fifo inode %" PRIu64 " != 0 (%" PRId64 " bytes)\n"), lino,
> -				(int64_t)be64_to_cpu(dino->di_size));
> -			break;
> -		case XR_INO_UQUOTA:
> -		case XR_INO_GQUOTA:
> -		case XR_INO_PQUOTA:
> -			do_warn(
> -_("size of quota inode %" PRIu64 " != 0 (%" PRId64 " bytes)\n"), lino,
> -				(int64_t)be64_to_cpu(dino->di_size));
> -			break;
> -		default:
> -			do_warn(_("Internal error - process_misc_ino_types, "
> -				  "illegal type %d\n"), type);
> -			abort();
> -		}
> -
> -		return(1);
> -	}
> -
> -	return(0);
> -}
> -
> -static int
> -process_misc_ino_types_blocks(
> -	xfs_rfsblock_t		totblocks,
> -	xfs_ino_t		lino,
> -	enum xr_ino_type	type)
> -{
> -	/*
> -	 * you can not enforce all misc types have zero data fork blocks
> -	 * by checking dino->di_nblocks because atotblocks (attribute
> -	 * blocks) are part of nblocks. We must check this later when atotblocks
> -	 * has been calculated or by doing a simple check that anExtents == 0.
> -	 * We must also guarantee that totblocks is 0. Thus nblocks checking
> -	 * will be done later in process_dinode_int for misc types.
> -	 */
> -
> -	if (totblocks != 0)  {
> -		switch (type)  {
> -		case XR_INO_CHRDEV:
> -			do_warn(
> -_("size of character device inode %" PRIu64 " != 0 (%" PRIu64 " blocks)\n"),
> -				lino, totblocks);
> -			break;
> -		case XR_INO_BLKDEV:
> -			do_warn(
> -_("size of block device inode %" PRIu64 " != 0 (%" PRIu64 " blocks)\n"),
> -				lino, totblocks);
> -			break;
> -		case XR_INO_SOCK:
> -			do_warn(
> -_("size of socket inode %" PRIu64 " != 0 (%" PRIu64 " blocks)\n"),
> -				lino, totblocks);
> -			break;
> -		case XR_INO_FIFO:
> -			do_warn(
> -_("size of fifo inode %" PRIu64 " != 0 (%" PRIu64 " blocks)\n"),
> -				lino, totblocks);
> -			break;
> -		default:
> -			return(0);
> -		}
> -		return(1);
> -	}
> -	return (0);
> -}
> -
>  static inline int
>  dinode_fmt(
>  	struct xfs_dinode *dino)
> @@ -2261,16 +2180,20 @@ _("directory inode %" PRIu64 " has bad size %" PRId64 "\n"),
>  	case XR_INO_BLKDEV:
>  	case XR_INO_SOCK:
>  	case XR_INO_FIFO:
> -		if (process_misc_ino_types(mp, dino, lino, type))
> -			return 1;
> -		break;
> -
>  	case XR_INO_UQUOTA:
>  	case XR_INO_GQUOTA:
>  	case XR_INO_PQUOTA:
> -		/* Quota inodes have same restrictions as above types */
> -		if (process_misc_ino_types(mp, dino, lino, type))
> +		/*
> +		 * Misc inode types that have no associated data storage (fifos,
> +		 * pipes, devices, etc.) mad thus must also have a zero size.
> +		 */
> +		if (dino->di_size != 0)  {
> +			do_warn(
> +_("size of %s inode %" PRIu64 " != 0 (%" PRId64 " bytes)\n"),
> +				xr_ino_type_name[type], lino,

i18n nit: In the old code, the inode description ("realtime bitmap")
would be translated, now they aren't.  I don't know of a good way to
maintain that though...

"el tamaño del realtime bitmap nodo 55 != 0"

               ^^^^^^^^^^^^^^^ whee, English in the middle of Spanish!

I guess you could write "_(xr_ino_type_name[type])" for that second
argument.

That aside, this is a lot less copy pasta at least.

--D

> +				(int64_t)be64_to_cpu(dino->di_size));
>  			return 1;
> +		}
>  		break;
>  
>  	case XR_INO_RTDATA:
> @@ -3704,10 +3627,26 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"),
>  	dino = *dinop;
>  
>  	/*
> -	 * enforce totblocks is 0 for misc types
> +	 * Enforce totblocks is 0 for misc types.
> +	 *
> +	 * Note that di_nblocks includes attribute fork blocks, so we can only
> +	 * do this here instead of when reading the inode.
>  	 */
> -	if (process_misc_ino_types_blocks(totblocks, lino, type))
> -		goto clear_bad_out;
> +	switch (type)  {
> +	case XR_INO_CHRDEV:
> +	case XR_INO_BLKDEV:
> +	case XR_INO_SOCK:
> +	case XR_INO_FIFO:
> +		if (totblocks != 0)  {
> +			do_warn(
> +_("size of %s inode %" PRIu64 " != 0 (%" PRIu64 " blocks)\n"),
> +				xr_ino_type_name[type], lino, totblocks);
> +			goto clear_bad_out;
> +		}
> +		break;
> +	default:
> +		break;
> +	}
>  
>  	/*
>  	 * correct space counters if required
> -- 
> 2.47.3
> 
>