Re: [PATCH 19/19] mkfs: add optional 'reason' for illegal_option

[Eric Sandeen <sandeen@sandeen.net>]

On 3/24/16 6:15 AM, jtulak@redhat.com wrote:
> From: Jan Tulak <jtulak@redhat.com>
> 
> Allow us to tell the user what exactly is wrong with his options.

"with the specified options" ;)

> For example, that the value is too small, instead of just generic
> "bad option."
> 
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> ---
>  mkfs/xfs_mkfs.c | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 680c6c4..76e193d 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1550,11 +1550,18 @@ static __attribute__((noreturn)) void
>  illegal_option(
>  	const char	*value,
>  	struct opt_params	*opts,
> -	int		index)
> +	int		index,
> +	const char *reason)
                   ^ tab that out to match.
>  {
> -	fprintf(stderr,
> -		_("Illegal value %s for -%c %s option\n"),
> -		value, opts->name, opts->subopts[index]);
> +	if(reason == NULL){
          ^space          ^space
> +		fprintf(stderr,
> +			_("Illegal value %s for -%c %s option\n"),
> +			value, opts->name, opts->subopts[index]);
> +	} else {
> +		fprintf(stderr,
> +			_("Illegal value %s for -%c %s option: %s\n"),
> +			value, opts->name, opts->subopts[index], reason);
> +	}

>  	usage();
>  }
> 
> @@ -1646,16 +1653,18 @@ getnum(
> 
>  		c = strtoll(str, &str_end, 0);
>  		if (c == 0 && str_end == str)
> -			illegal_option(str, opts, index);
> +			illegal_option(str, opts, index, NULL);
>  		if (*str_end != '\0')
> -			illegal_option(str, opts, index);
> +			illegal_option(str, opts, index, NULL);
>  	}
> 
>  	/* Validity check the result. */
> -	if (c < sp->minval || c > sp->maxval)
> -		illegal_option(str, opts, index);
> +	if (c < sp->minval)
> +		illegal_option(str, opts, index, "value is too small");
> +	else if (c > sp->maxval)
> +		illegal_option(str, opts, index, "value is too large");
>  	if (sp->is_power_2 && !ispow2(c))
> -		illegal_option(str, opts, index);
> +		illegal_option(str, opts, index, "value has to be power of 2");

                                                 "value must be a power of 2"


The "reason" strings should be translatable strings too, right? :  _(" ... ")

-Eric

>  	return c;
>  }
> 
> --
> 2.6.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs