[PATCH] programs/sc: Allow using separate arguments for parameter name and value.
In addition to using one argument for both parameter name and value. This fixes a regression from commit 8b38c91d83844dea882922055ced7cdeb79c1693. Signed-off-by: Torge Matthies <tmatthies(a)codeweavers.com> --- programs/sc/sc.c | 101 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 69 insertions(+), 32 deletions(-) diff --git a/programs/sc/sc.c b/programs/sc/sc.c index 43148ab99d60..5caffdbe7fe4 100644 --- a/programs/sc/sc.c +++ b/programs/sc/sc.c @@ -27,6 +27,26 @@ WINE_DEFAULT_DEBUG_CHANNEL(sc); +static BOOL parse_string_param( int argc, const WCHAR *argv[], unsigned int *index, + const WCHAR *param_name, size_t name_len, const WCHAR **out ) +{ + if (!wcsnicmp( argv[*index], param_name, name_len )) + { + if (argv[*index][name_len]) + { + *out = &argv[*index][name_len]; + return TRUE; + } + else if (*index < argc - 1) + { + *index += 1; + *out = argv[*index]; + return TRUE; + } + } + return FALSE; +} + struct create_params { const WCHAR *displayname; @@ -56,48 +76,56 @@ static BOOL parse_create_params( int argc, const WCHAR *argv[], struct create_pa cp->obj = NULL; cp->password = NULL; +#define PARSE(x, y) parse_string_param( (argc), (argv), &(i), (x), ARRAY_SIZE(x) - 1, (y) ) for (i = 0; i < argc; i++) { - 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 (!wcsnicmp( argv[i], L"tag=", 4 )) + const WCHAR *tag, *type, *start, *error; + + if (PARSE( L"displayname=", &cp->displayname )) continue; + if (PARSE( L"binpath=", &cp->binpath )) continue; + if (PARSE( L"group=", &cp->group )) continue; + if (PARSE( L"depend=", &cp->depend )) continue; + if (PARSE( L"obj=", &cp->obj )) continue; + if (PARSE( L"password=", &cp->password )) continue; + + if (PARSE( L"tag=", &tag )) { - if (!wcsicmp( argv[i] + 4, L"yes" )) + if (!wcsicmp( tag, L"yes" )) { WINE_FIXME("tag argument not supported\n"); cp->tag = TRUE; } + continue; } - if (!wcsnicmp( argv[i], L"type=", 5 )) + if (PARSE( L"type=", &type )) { - if (!wcsicmp( argv[i] + 5, L"own" )) cp->type = SERVICE_WIN32_OWN_PROCESS; - if (!wcsicmp( argv[i] + 5, L"share" )) cp->type = SERVICE_WIN32_SHARE_PROCESS; - if (!wcsicmp( argv[i] + 5, L"kernel" )) cp->type = SERVICE_KERNEL_DRIVER; - if (!wcsicmp( argv[i] + 5, L"filesys" )) cp->type = SERVICE_FILE_SYSTEM_DRIVER; - if (!wcsicmp( argv[i] + 5, L"rec" )) cp->type = SERVICE_RECOGNIZER_DRIVER; - if (!wcsicmp( argv[i] + 5, L"interact" )) cp->type |= SERVICE_INTERACTIVE_PROCESS; + if (!wcsicmp( type, L"own" )) cp->type = SERVICE_WIN32_OWN_PROCESS; + else if (!wcsicmp( type, L"share" )) cp->type = SERVICE_WIN32_SHARE_PROCESS; + else if (!wcsicmp( type, L"kernel" )) cp->type = SERVICE_KERNEL_DRIVER; + else if (!wcsicmp( type, L"filesys" )) cp->type = SERVICE_FILE_SYSTEM_DRIVER; + else if (!wcsicmp( type, L"rec" )) cp->type = SERVICE_RECOGNIZER_DRIVER; + else if (!wcsicmp( type, L"interact" )) cp->type |= SERVICE_INTERACTIVE_PROCESS; + continue; } - if (!wcsnicmp( argv[i], L"start=", 6 )) + if (PARSE( L"start=", &start )) { - if (!wcsicmp( argv[i] + 6, L"boot" )) cp->start = SERVICE_BOOT_START; - if (!wcsicmp( argv[i] + 6, L"system" )) cp->start = SERVICE_SYSTEM_START; - if (!wcsicmp( argv[i] + 6, L"auto" )) cp->start = SERVICE_AUTO_START; - if (!wcsicmp( argv[i] + 6, L"demand" )) cp->start = SERVICE_DEMAND_START; - if (!wcsicmp( argv[i] + 6, L"disabled" )) cp->start = SERVICE_DISABLED; + if (!wcsicmp( start, L"boot" )) cp->start = SERVICE_BOOT_START; + else if (!wcsicmp( start, L"system" )) cp->start = SERVICE_SYSTEM_START; + else if (!wcsicmp( start, L"auto" )) cp->start = SERVICE_AUTO_START; + else if (!wcsicmp( start, L"demand" )) cp->start = SERVICE_DEMAND_START; + else if (!wcsicmp( start, L"disabled" )) cp->start = SERVICE_DISABLED; + continue; } - if (!wcsnicmp( argv[i], L"error=", 6 )) + if (PARSE( L"error=", &error )) { - if (!wcsicmp( argv[i] + 6, L"normal" )) cp->error = SERVICE_ERROR_NORMAL; - if (!wcsicmp( argv[i] + 6, L"severe" )) cp->error = SERVICE_ERROR_SEVERE; - if (!wcsicmp( argv[i] + 6, L"critical" )) cp->error = SERVICE_ERROR_CRITICAL; - if (!wcsicmp( argv[i] + 6, L"ignore" )) cp->error = SERVICE_ERROR_IGNORE; + if (!wcsicmp( error, L"normal" )) cp->error = SERVICE_ERROR_NORMAL; + else if (!wcsicmp( error, L"severe" )) cp->error = SERVICE_ERROR_SEVERE; + else if (!wcsicmp( error, L"critical" )) cp->error = SERVICE_ERROR_CRITICAL; + else if (!wcsicmp( error, L"ignore" )) cp->error = SERVICE_ERROR_IGNORE; + continue; } } +#undef PARSE if (!cp->binpath) return FALSE; return TRUE; } @@ -156,16 +184,25 @@ static BOOL parse_failure_params( int argc, const WCHAR *argv[], SERVICE_FAILURE fa->cActions = 0; fa->lpsaActions = NULL; +#define PARSE(x, y) parse_string_param( (argc), (argv), &(i), (x), ARRAY_SIZE(x) - 1, (y) ) for (i = 0; i < argc; i++) { - if (!wcsnicmp( argv[i], L"reset=", 6 )) fa->dwResetPeriod = wcstol( argv[i] + 6, NULL, 10 ); - if (!wcsnicmp( argv[i], L"reboot=", 7 )) fa->lpRebootMsg = (WCHAR *)argv[i] + 7; - if (!wcsnicmp( argv[i], L"command=", 8 )) fa->lpCommand = (WCHAR *)argv[i] + 8; - if (!wcsnicmp( argv[i], L"actions=", 8 )) + const WCHAR *reset, *actions; + + if (PARSE( L"reset=", &reset )) + { + fa->dwResetPeriod = wcstol( reset, NULL, 10 ); + continue; + } + if (PARSE( L"reboot=", &fa->lpRebootMsg )) continue; + if (PARSE( L"command=", &fa->lpCommand )) continue; + if (PARSE( L"actions=", &actions )) { - if (!parse_failure_actions( argv[i] + 8, fa )) return FALSE; + if (!parse_failure_actions( actions, fa )) return FALSE; + continue; } } +#undef PARSE return TRUE; } -- 2.36.0
Torge Matthies <tmatthies(a)codeweavers.com> wrote:
In addition to using one argument for both parameter name and value.
This fixes a regression from commit 8b38c91d83844dea882922055ced7cdeb79c1693.
Could you please clarify what precisely is the regression caused by the mentioned above commit, and provide some examples which command lines were broken and started to work with your patch. -- Dmitry.
On 4/25/22 20:17, Dmitry Timoshkov wrote:
Torge Matthies <tmatthies(a)codeweavers.com> wrote:
In addition to using one argument for both parameter name and value.
This fixes a regression from commit 8b38c91d83844dea882922055ced7cdeb79c1693.
Could you please clarify what precisely is the regression caused by the mentioned above commit, and provide some examples which command lines were broken and started to work with your patch.
The mentioned commit broke commands like: sc.exe create test binpath= C:\windows\system32\cmd.exe and: sc.exe create test binpath= C:\windows\system32\cmd.exe displayname= "Test Service" start= auto All sc create or sc failure commands where the parameter names and values are separated by a space are affected. Should I add the commands above to the commit message?
Torge Matthies <tmatthies(a)codeweavers.com> writes:
On 4/25/22 20:17, Dmitry Timoshkov wrote:
Torge Matthies <tmatthies(a)codeweavers.com> wrote:
In addition to using one argument for both parameter name and value.
This fixes a regression from commit 8b38c91d83844dea882922055ced7cdeb79c1693. Could you please clarify what precisely is the regression caused by the mentioned above commit, and provide some examples which command lines were broken and started to work with your patch.
The mentioned commit broke commands like:
sc.exe create test binpath= C:\windows\system32\cmd.exe
and:
sc.exe create test binpath= C:\windows\system32\cmd.exe displayname= "Test Service" start= auto
All sc create or sc failure commands where the parameter names and values are separated by a space are affected.
Should I add the commands above to the commit message?
If possible, adding some test cases would be preferable. -- Alexandre Julliard julliard(a)winehq.org
Torge Matthies <tmatthies(a)codeweavers.com> wrote:
On 4/25/22 20:17, Dmitry Timoshkov wrote:
Torge Matthies <tmatthies(a)codeweavers.com> wrote:
In addition to using one argument for both parameter name and value.
This fixes a regression from commit 8b38c91d83844dea882922055ced7cdeb79c1693.
Could you please clarify what precisely is the regression caused by the mentioned above commit, and provide some examples which command lines were broken and started to work with your patch.
The mentioned commit broke commands like:
sc.exe create test binpath= C:\windows\system32\cmd.exe
and:
sc.exe create test binpath= C:\windows\system32\cmd.exe displayname= "Test Service" start= auto
All sc create or sc failure commands where the parameter names and values are separated by a space are affected.
Thanks, it looks like sc.exe from Windows 10 does support providing an argument as a separate parameter, and it's even reflected in the help message (parameter name includes equal sign (=)) when 'binpath = path' is passed on the command line. Just curious, is your patch supposed to fix some real use case? -- Dmitry.
On 4/25/22 20:52, Dmitry Timoshkov wrote:
Torge Matthies <tmatthies(a)codeweavers.com> wrote:
On 4/25/22 20:17, Dmitry Timoshkov wrote:
Torge Matthies <tmatthies(a)codeweavers.com> wrote:
In addition to using one argument for both parameter name and value.
This fixes a regression from commit 8b38c91d83844dea882922055ced7cdeb79c1693.
Could you please clarify what precisely is the regression caused by the mentioned above commit, and provide some examples which command lines were broken and started to work with your patch.
The mentioned commit broke commands like:
sc.exe create test binpath= C:\windows\system32\cmd.exe
and:
sc.exe create test binpath= C:\windows\system32\cmd.exe displayname= "Test Service" start= auto
All sc create or sc failure commands where the parameter names and values are separated by a space are affected.
Thanks, it looks like sc.exe from Windows 10 does support providing an argument as a separate parameter, and it's even reflected in the help message (parameter name includes equal sign (=)) when 'binpath = path' is passed on the command line.
Just curious, is your patch supposed to fix some real use case?
It's supposed to fix service creation with sc.exe in general. The syntax with the spaces has been the default forever and I didn't even know that the other syntax was accepted. I have a script that installs an exe as a service and that got broken. But I'm sure there are many more things that depend on sc.exe accepting the syntax with spaces between parameter name and value.
participants (3)
-
Alexandre Julliard -
Dmitry Timoshkov -
Torge Matthies