[PATCH] xkbcomp: Improved -w option parsing

[Peter Hutterer]

On Mon, Aug 04, 2014 at 03:27:49PM +0200, Vincent Lefevre wrote:
> This patch improves -w option parsing even further, for cases like
> "xkbcomp -w6 4.xkb out.xkb" (which were not handled by the fix of
> #66344). Moreover, though this form can be regarded as ambiguous,
> the warning level is still optional (set to 0 if not present), and
> errors like "xkbcomp -wfoo in out" are detected and reported.
> 
> Signed-off-by: Vincent Lefevre <vincent at vinc17.net>
> ---
>  xkbcomp.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/xkbcomp.c b/xkbcomp.c
> index 956e79c..1bb8ab2 100644
> --- a/xkbcomp.c
> +++ b/xkbcomp.c
> @@ -576,17 +576,33 @@ parseArgs(int argc, char *argv[])
>          }
>          else if (strncmp(argv[i], "-w", 2) == 0)
>          {
> -            if ((i >= (argc - 1)) || (!isdigit(argv[i + 1][0])))
> +            unsigned long utmp;
> +            char *tmp2;
> +            /* If text is just after "-w" in the same word, then it must
> +             * be a number and it is the warning level. Otherwise, if the
> +             * next argument is a number, then it is the warning level,
> +             * else the warning level is assumed to be 0.
> +             */
> +            if (argv[i][2] == '\0')
>              {
>                  warningLevel = 0;
> -                if (isdigit(argv[i][2]))
> -                    if (sscanf(&argv[i][2], "%i", &itmp) == 1)
> -                        warningLevel = itmp;
> +                if (i < argc - 1)
> +                {
> +                    utmp = strtoul(argv[i+1], &tmp2, 10);
> +                    if (argv[i+1][0] != '\0' && *tmp2 == '\0')
> +                    {
> +                        warningLevel = utmp > 10 ? 10 : utmp;
> +                        i++;
> +                    }
> +                }
>              }
>              else
>              {
> -                if (sscanf(argv[++i], "%i", &itmp) == 1)
> -                    warningLevel = itmp;
> +                utmp = strtoul(&argv[i][2], &tmp2, 10);
> +                if (*tmp2 == '\0')
> +                    warningLevel = utmp > 10 ? 10 : utmp;
> +                else
> +                    goto unknown_flag;

whoah, this is pretty much a perfect example of how not to use goto...
adding three lines for error and exist isn't that hard, jumping from one
random spot into another random spot is not ok.

rest looks good though, thanks.

Cheers,
   Peter

>              }
>          }
>          else if ((strcmp(argv[i], "-xkb") == 0) && (!xkblist))
> @@ -619,6 +635,7 @@ parseArgs(int argc, char *argv[])
>          }
>          else
>          {
> +          unknown_flag:
>              ERROR1("Unknown flag \"%s\" on command line\n", argv[i]);
>              Usage(argc, argv);
>              return False;
> -- 
> 2.0.1
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>