xfs
[Top] [All Lists]

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

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH 19/19] mkfs: add optional 'reason' for illegal_option
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Wed, 6 Apr 2016 17:23:32 -0500
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1458818136-56043-20-git-send-email-jtulak@xxxxxxxxxx>
References: <1458818136-56043-1-git-send-email-jtulak@xxxxxxxxxx> <1458818136-56043-20-git-send-email-jtulak@xxxxxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:38.0) Gecko/20100101 Thunderbird/38.7.1
On 3/24/16 6:15 AM, jtulak@xxxxxxxxxx wrote:
> From: Jan Tulak <jtulak@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [PATCH 19/19] mkfs: add optional 'reason' for illegal_option, Eric Sandeen <=