Re: [PATCH 2/4] xfs: explicitly initialize meta_scrub_ops array by type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 01, 2017 at 04:13:27PM -0600, Eric Sandeen wrote:
> An implicit mapping to type by order of initialization seems
> error-prone, and doesn't lend itself to cscope-ing.
> 
> Also add sanity checks about size of array vs. max types,
> and a defensive check that ->scrub exists before using it.
> 
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>

> ---
> 
> diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c
> index 9c42c4e..dc1f33a 100644
> --- a/fs/xfs/scrub/scrub.c
> +++ b/fs/xfs/scrub/scrub.c
> @@ -168,104 +168,104 @@
>  /* Scrubbing dispatch. */
>  
>  static const struct xfs_scrub_meta_ops meta_scrub_ops[] = {
> -	{ /* ioctl presence test */
> +	[XFS_SCRUB_TYPE_PROBE] = {	/* ioctl presence test */
>  		.setup	= xfs_scrub_setup_fs,
>  		.scrub	= xfs_scrub_probe,
>  	},
> -	{ /* superblock */
> +	[XFS_SCRUB_TYPE_SB] = {		/* superblock */
>  		.setup	= xfs_scrub_setup_ag_header,
>  		.scrub	= xfs_scrub_superblock,
>  	},
> -	{ /* agf */
> +	[XFS_SCRUB_TYPE_AGF] = {	/* agf */
>  		.setup	= xfs_scrub_setup_ag_header,
>  		.scrub	= xfs_scrub_agf,
>  	},
> -	{ /* agfl */
> +	[XFS_SCRUB_TYPE_AGFL]= {	/* agfl */
>  		.setup	= xfs_scrub_setup_ag_header,
>  		.scrub	= xfs_scrub_agfl,
>  	},
> -	{ /* agi */
> +	[XFS_SCRUB_TYPE_AGI] = {	/* agi */
>  		.setup	= xfs_scrub_setup_ag_header,
>  		.scrub	= xfs_scrub_agi,
>  	},
> -	{ /* bnobt */
> +	[XFS_SCRUB_TYPE_BNOBT] = {	/* bnobt */
>  		.setup	= xfs_scrub_setup_ag_allocbt,
>  		.scrub	= xfs_scrub_bnobt,
>  	},
> -	{ /* cntbt */
> +	[XFS_SCRUB_TYPE_CNTBT] = {	/* cntbt */
>  		.setup	= xfs_scrub_setup_ag_allocbt,
>  		.scrub	= xfs_scrub_cntbt,
>  	},
> -	{ /* inobt */
> +	[XFS_SCRUB_TYPE_INOBT] = {	/* inobt */
>  		.setup	= xfs_scrub_setup_ag_iallocbt,
>  		.scrub	= xfs_scrub_inobt,
>  	},
> -	{ /* finobt */
> +	[XFS_SCRUB_TYPE_FINOBT] = {	/* finobt */
>  		.setup	= xfs_scrub_setup_ag_iallocbt,
>  		.scrub	= xfs_scrub_finobt,
>  		.has	= xfs_sb_version_hasfinobt,
>  	},
> -	{ /* rmapbt */
> +	[XFS_SCRUB_TYPE_RMAPBT] = {	/* rmapbt */
>  		.setup	= xfs_scrub_setup_ag_rmapbt,
>  		.scrub	= xfs_scrub_rmapbt,
>  		.has	= xfs_sb_version_hasrmapbt,
>  	},
> -	{ /* refcountbt */
> +	[XFS_SCRUB_TYPE_REFCNTBT] = {	/* refcountbt */
>  		.setup	= xfs_scrub_setup_ag_refcountbt,
>  		.scrub	= xfs_scrub_refcountbt,
>  		.has	= xfs_sb_version_hasreflink,
>  	},
> -	{ /* inode record */
> +	[XFS_SCRUB_TYPE_INODE] = {	/* inode record */
>  		.setup	= xfs_scrub_setup_inode,
>  		.scrub	= xfs_scrub_inode,
>  	},
> -	{ /* inode data fork */
> +	[XFS_SCRUB_TYPE_BMBTD] = {	/* inode data fork */
>  		.setup	= xfs_scrub_setup_inode_bmap,
>  		.scrub	= xfs_scrub_bmap_data,
>  	},
> -	{ /* inode attr fork */
> +	[XFS_SCRUB_TYPE_BMBTA] = {	/* inode attr fork */
>  		.setup	= xfs_scrub_setup_inode_bmap,
>  		.scrub	= xfs_scrub_bmap_attr,
>  	},
> -	{ /* inode CoW fork */
> +	[XFS_SCRUB_TYPE_BMBTC] = {	/* inode CoW fork */
>  		.setup	= xfs_scrub_setup_inode_bmap,
>  		.scrub	= xfs_scrub_bmap_cow,
>  	},
> -	{ /* directory */
> +	[XFS_SCRUB_TYPE_DIR] = {	/* directory */
>  		.setup	= xfs_scrub_setup_directory,
>  		.scrub	= xfs_scrub_directory,
>  	},
> -	{ /* extended attributes */
> +	[XFS_SCRUB_TYPE_XATTR] = {	/* extended attributes */
>  		.setup	= xfs_scrub_setup_xattr,
>  		.scrub	= xfs_scrub_xattr,
>  	},
> -	{ /* symbolic link */
> +	[XFS_SCRUB_TYPE_SYMLINK] = {	/* symbolic link */
>  		.setup	= xfs_scrub_setup_symlink,
>  		.scrub	= xfs_scrub_symlink,
>  	},
> -	{ /* parent pointers */
> +	[XFS_SCRUB_TYPE_PARENT] = {	/* parent pointers */
>  		.setup	= xfs_scrub_setup_parent,
>  		.scrub	= xfs_scrub_parent,
>  	},
> -	{ /* realtime bitmap */
> +	[XFS_SCRUB_TYPE_RTBITMAP] = {	/* realtime bitmap */
>  		.setup	= xfs_scrub_setup_rt,
>  		.scrub	= xfs_scrub_rtbitmap,
>  		.has	= xfs_sb_version_hasrealtime,
>  	},
> -	{ /* realtime summary */
> +	[XFS_SCRUB_TYPE_RTSUM] = {	/* realtime summary */
>  		.setup	= xfs_scrub_setup_rt,
>  		.scrub	= xfs_scrub_rtsummary,
>  		.has	= xfs_sb_version_hasrealtime,
>  	},
> -	{ /* user quota */
> +	[XFS_SCRUB_TYPE_UQUOTA] = {	/* user quota */
>  		.setup = xfs_scrub_setup_quota,
>  		.scrub = xfs_scrub_quota,
>  	},
> -	{ /* group quota */
> +	[XFS_SCRUB_TYPE_GQUOTA] = {	/* group quota */
>  		.setup = xfs_scrub_setup_quota,
>  		.scrub = xfs_scrub_quota,
>  	},
> -	{ /* project quota */
> +	[XFS_SCRUB_TYPE_PQUOTA] = {	/* project quota */
>  		.setup = xfs_scrub_setup_quota,
>  		.scrub = xfs_scrub_quota,
>  	},
> @@ -297,6 +297,9 @@
>  	bool				try_harder = false;
>  	int				error = 0;
>  
> +	BUILD_BUG_ON(sizeof(meta_scrub_ops) !=
> +		(sizeof(struct xfs_scrub_meta_ops) * XFS_SCRUB_TYPE_NR));
> +
>  	trace_xfs_scrub_start(ip, sm, error);
>  
>  	/* Forbidden if we are shut down or mounted norecovery. */
> @@ -320,7 +323,7 @@
>  	if (sm->sm_type >= XFS_SCRUB_TYPE_NR)
>  		goto out;
>  	ops = &meta_scrub_ops[sm->sm_type];
> -	if (ops->scrub == NULL)
> +	if (ops->setup == NULL || ops->scrub == NULL)
>  		goto out;
>  
>  	/*
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux