Re: [PATCH 1/2] sc: Fix parsing parameter name in the 'switch=param' constructs.

[Emil Velikov]

Hi Dmitry,

On Fri, 18 Mar 2022 at 14:11, Dmitry Timoshkov  wrote:
> So, that 'sc create test binpath=test.exe' actually works.
> Signed-off-by: Dmitry Timoshkov
>
> programs/sc/sc.c | 63 ++++++++++++++++++++++++------------------------
>  1 file changed, 31 insertions(+), 32 deletions(-)
>
> diff --git a/programs/sc/sc.c b/programs/sc/sc.c
> index 6cf27d93ac2..bd3064da2d8 100644
> --- a/programs/sc/sc.c
> +++ b/programs/sc/sc.c
> @@ -58,44 +58,44 @@ static BOOL parse_create_params( int argc, const WCHAR *argv[], struct create_pa
>  for (i = 0; i < argc; i++)
>  {
> -
>    if (!wcsicmp( argv[i], L"displayname=" ) && i < argc - 1) cp->displayname = argv[i + 1];
> -
>    if (!wcsicmp( argv[i], L"binpath=" ) && i < argc - 1) cp->binpath = argv[i + 1];
> -
>    if (!wcsicmp( argv[i], L"group=" ) && i < argc - 1) cp->group = argv[i + 1];
> -
>    if (!wcsicmp( argv[i], L"depend=" ) && i < argc - 1) cp->depend = argv[i + 1];
> -
>    if (!wcsicmp( argv[i], L"obj=" ) && i < argc - 1) cp->obj = argv[i + 1];
> -
>    if (!wcsicmp( argv[i], L"password=" ) && i < argc - 1) cp->password = argv[i + 1];
> -
>    if (!wcsnicmp( argv[i], L"displayname=", 12 )) cp->displayname = argv[i] + 12;
> -
>    if (!wcsnicmp( argv[i], L"binpath=", 8 )) cp->binpath = argv[i] + 8;
> -
>    if (!wcsnicmp( argv[i], L"group=", 6 )) cp->group = argv[i] + 6;
> -
>    if (!wcsnicmp( argv[i], L"depend=", 7 )) cp->depend = argv[i] + 7;
> -
>    if (!wcsnicmp( argv[i], L"obj=", 4 )) cp->obj = argv[i] + 4;
> -
>    if (!wcsnicmp( argv[i], L"password=", 9 )) cp->password = argv[i] + 9;
>
> -
>    if (!wcsicmp( argv[i], L"tag=" ) && i < argc - 1)
> -
>    if (!wcsnicmp( argv[i], L"tag=", 4 ))
>    {
> -
>        if (!wcsicmp( argv[i], L"yes" ))
> -
>        if (!wcsicmp( argv[i] + 4, L"yes" ))
>        {
>            WINE_FIXME("tag argument not supported\n");
>            cp->tag = TRUE;

The proposed patch fixes the original issue, although the parsing feels off.
In particular: we silently accept any empty/invalid attribute for
tag/type/start...

I tend to split the parsing and conversion/validation wrapping the lot
into a macro like below.
In this case one needs to change the type of
cp->{tag,type,start,error} to WCHAR * + add new variables for the
original data.

#define parse_argv(_arg) \
    do { \
        static const WCHAR *_larg = L ### _arg "="; \
        static const unsigned _size = wcslen(_larg); \
        if (!wcsnicmp( argv[i], _larg, _size )) { \
            cp-> _arg = argv[i] + _size; \
            validate_ # _arg(cp); \
        } \
    } while (0)

Then the function becomes short and sweet:
    parse_argv(displayname);
    parse_argv(binpath);
    ....
    parse_argv(type);
    ...

Hope it helps
-Emil