Re: [TRY 2] [2/3] extrac32: Add command-line parsing

[Alexandre Julliard]

Ilya Shpigor <shpigor(a)etersoft.ru> writes:
> +    /* Parse options */
> +    for(i = 0; i < argc; i++)
> +    {
> +        if (argv[i][0] == '/')
> +        {
> +            for(j = 0; j < sizeof(sFlags) / sizeof(sFlags[0]); j++)
> +                if (!strcmpiW(argv[i], sFlags[j].wFlag))
> +                {
> +                    comp |= sFlags[j].nFlag;
> +                    goto next;
> +                }
> +        }
> +        next:
> +            continue;
> +    }
> +
> +    if ((comp & CMD_FLAGS) == EFLAGC) /* copy file */
> +    {
> +        WINE_FIXME("/C not implemented\n");
> +    } else if ((comp & CMD_FLAGS) == EFLAGD) /* display CAB archive */
> +    {
> +        WINE_FIXME("/D not implemented\n");
> +    } else if ((comp & CMD_FLAGS) == EFLAGE) /* extract CAB archive */
> +    {
> +        WINE_FIXME("/E not implemented\n");
> +    } else if ((comp & CMD_FLAGS) == 0) /* expand mode */
> +    {
> +        WINE_FIXME("Expand mode not implemented\n");
> +    }

That parsing approach is not really appropriate here, the options are
not just a bunch of flags, there are options with arguments, mutually
exclusive modes, etc. Some sort of switch() statement would be more
natural.

-- 
Alexandre Julliard
julliard(a)winehq.org

[TRY 2] [2/3] extrac32: Add command-line parsing

[Andrey Turkin]

Alexandre Julliard wrote:
> Ilya Shpigor <shpigor(a)etersoft.ru> writes:
> > +    /* Parse options */
> > +    for(i = 0; i < argc; i++)
> > +    {
> > +        if (argv[i][0] == '/')
> > +        {
> > +            for(j = 0; j < sizeof(sFlags) / sizeof(sFlags[0]); j++)
> > +                if (!strcmpiW(argv[i], sFlags[j].wFlag))
> > +                {
> > +                    comp |= sFlags[j].nFlag;
> > +                    goto next;
> > +                }
> > +        }
> > +        next:
> > +            continue;
> > +    }
> > +
> > +    if ((comp & CMD_FLAGS) == EFLAGC) /* copy file */
> > +    {
> > +        WINE_FIXME("/C not implemented\n");
> > +    } else if ((comp & CMD_FLAGS) == EFLAGD) /* display CAB archive */
> > +    {
> > +        WINE_FIXME("/D not implemented\n");
> > +    } else if ((comp & CMD_FLAGS) == EFLAGE) /* extract CAB archive */
> > +    {
> > +        WINE_FIXME("/E not implemented\n");
> > +    } else if ((comp & CMD_FLAGS) == 0) /* expand mode */
> > +    {
> > +        WINE_FIXME("Expand mode not implemented\n");
> > +    }
>
> That parsing approach is not really appropriate here, the options are
> not just a bunch of flags, there are options with arguments, mutually
> exclusive modes, etc. Some sort of switch() statement would be more
> natural.

Actually they are just a bunch of flags with exception for /L which 
takes an argument - this is covered by Ilya's third patch, and /C, /D 
and /E which are mutually exclusive "mode" switches - this is covered by 
CMD_FLAGS bitmask. At least Vista's extrac32 don't care for 
inappropriate flags, e.g.:
> md temp
> md temp2
> echo "test" > test
> extrac32 /Y /A /L temp2 /C test temp|more
> Microsoft (R) Cabinet Extraction Tool - Version 6.0.6001.18000
> Copyright (c) Microsoft Corporation. All rights reserved..
> Extracting test -> temp\test

And indeed there is a copied file in "temp" directory despite erroneous 
"/A" and "/L path" flags.