Re: [PATCH] xfs_quota: refactor code to generate id from name

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

On Sat, May 09, 2020 at 12:18:42PM -0500, Eric Sandeen wrote:
> There's boilerplate for setting limits and warnings, where we have
> a case statement for each of the 3 quota types, and from there call
> 3 different functions to configure each of the 3 types, each of which
> calls its own version of id to string function... 
> 
> Refactor this so that the main function can call a generic id to string
> conversion routine, and then call a common action.  This save a lot of
> LOC.
> 
> I was looking at allowing xfs to bump out individual grace periods like
> setquota can do, and this refactoring allows us to add new actions like
> that without copyingall the boilerplate again.
> 
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
> 
>  edit.c |  196 ++++++++++++++++-----------------------------------------
>  1 file changed, 51 insertions(+), 145 deletions(-)
> 
> diff --git a/quota/edit.c b/quota/edit.c
> index f9938b8a..70c0969f 100644
> --- a/quota/edit.c
> +++ b/quota/edit.c
> @@ -101,6 +101,42 @@ warn_help(void)
>  "\n"));
>  }
>  
> +static uint32_t
> +id_from_string(
> +	char	*name,
> +	int	type)
> +{
> +	uint32_t	id = -1;
> +
> +	switch (type) {
> +	case XFS_USER_QUOTA:
> +		id = uid_from_string(name);
> +		if (id == -1)
> +			fprintf(stderr, _("%s: invalid user name: %s\n"),
> +				progname, name);
> +		break;
> +	case XFS_GROUP_QUOTA:
> +		id = gid_from_string(name);
> +		if (id == -1)
> +			fprintf(stderr, _("%s: invalid group name: %s\n"),
> +				progname, name);
> +		break;
> +	case XFS_PROJ_QUOTA:
> +		id = prid_from_string(name);
> +		if (id == -1)
> +			fprintf(stderr, _("%s: invalid project name: %s\n"),
> +				progname, name);
> +		break;
> +	default:
> +		ASSERT(0);
> +		break;
> +	}
> +
> +	if (id == -1)
> +		exitcode = 1;
> +	return id;

What about de-duplicating the error printk as well?

static uint32_t
id_from_string(
	char		*name,
	int		type)
{
	uint32_t	id = -1;
	const char	*type = "invalid";

	switch (type) {
	case XFS_USER_QUOTA:
		type = "user";
		id = uid_from_string(name);
		break;
	case XFS_GROUP_QUOTA:
		type = "group";
		id = gid_from_string(name);
		break;
	case XFS_PROJ_QUOTA:
		type = "project";
		id = prid_from_string(name);
		break;
	default:
		ASSERT(0);
		break;
	}

	if (id == -1) {
		fprintf(stderr, _("%s: invalid %s name: %s\n"),
			type, progname, name);
		exitcode = 1;
	}
	return id;
}