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

Eric Sandeen
2016-04-06T22:23:36Z

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