[PATCH 0/1] MR6476: sc: Return error value rather than 1 on failure.

[Owen Rudge (＠orudge)]

The Windows version of `sc.exe` returns the appropriate error value, which some applications rely on if programmatically executing it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6476

[PATCH 1/1] sc: Return error value rather than 1 on failure.

[Owen Rudge]

From: Owen Rudge <orudge(a)codeweavers.com>

---
 programs/sc/sc.c       | 116 +++++++++++++++++++++++++++++------------
 programs/sc/tests/sc.c |   8 ++-
 2 files changed, 86 insertions(+), 38 deletions(-)

diff --git a/programs/sc/sc.c b/programs/sc/sc.c
index d1954da1493..7c271a6aa8e 100644
--- a/programs/sc/sc.c
+++ b/programs/sc/sc.c
@@ -208,7 +208,7 @@ static BOOL parse_failure_params( int argc, const WCHAR *argv[], SERVICE_FAILURE
 static void usage( void )
 {
     WINE_MESSAGE( "Usage: sc command servicename [parameter= value ...]\n" );
-    exit( 1 );
+    exit( ERROR_INVALID_COMMAND_LINE );
 }
 
 static const WCHAR *service_type_string( DWORD type )
@@ -231,18 +231,20 @@ static const WCHAR *service_state_string( DWORD state )
     return L"";
 }
 
-static BOOL query_service( SC_HANDLE manager, const WCHAR *name )
+static DWORD query_service( SC_HANDLE manager, const WCHAR *name )
 {
     SC_HANDLE service;
     SERVICE_STATUS status;
-    BOOL ret = FALSE;
+    DWORD ret = ERROR_SUCCESS;
 
     service = OpenServiceW( manager, name, SERVICE_QUERY_STATUS );
     if (service)
     {
-        ret = QueryServiceStatus( service, &status );
-        if (!ret)
-            WINE_ERR("failed to query service status %lu\n", GetLastError());
+        if (!QueryServiceStatus( service, &status ))
+        {
+            ret = GetLastError();
+            WINE_ERR("failed to query service status %lu\n", ret);
+        }
         else
             printf( "SERVICE_NAME: %ls\n"
                     "        TYPE               : %lx  %ls\n"
@@ -258,7 +260,11 @@ static BOOL query_service( SC_HANDLE manager, const WCHAR *name )
                     status.dwCheckPoint, status.dwWaitHint );
         CloseServiceHandle( service );
     }
-    else WINE_ERR("failed to open service %lu\n", GetLastError());
+    else
+    {
+        ret = GetLastError();
+        WINE_ERR("failed to open service %lu\n", ret);
+    }
 
     return ret;
 }
@@ -267,21 +273,22 @@ int __cdecl wmain( int argc, const WCHAR *argv[] )
 {
     SC_HANDLE manager, service;
     SERVICE_STATUS status;
-    BOOL ret = FALSE;
+    DWORD ret = ERROR_SUCCESS;
 
     if (argc < 3) usage();
 
     if (argv[2][0] == '\\' && argv[2][1] == '\\')
     {
         WINE_FIXME("server argument not supported\n");
-        return 1;
+        return ERROR_INVALID_COMMAND_LINE;
     }
 
     manager = OpenSCManagerW( NULL, NULL, SC_MANAGER_ALL_ACCESS );
     if (!manager)
     {
-        WINE_ERR("failed to open service manager\n");
-        return 1;
+        ret = GetLastError();
+        WINE_ERR("failed to open service manager: %lu\n", ret);
+        return ret;
     }
 
     if (!wcsicmp( argv[1], L"create" ))
@@ -297,7 +304,7 @@ int __cdecl wmain( int argc, const WCHAR *argv[] )
         {
             WINE_ERR("failed to parse create parameters\n");
             CloseServiceHandle( manager );
-            return 1;
+            return ret;
         }
         service = CreateServiceW( manager, argv[2], cp.displayname, SERVICE_ALL_ACCESS,
                                   cp.type, cp.start, cp.error, cp.binpath, cp.group, NULL,
@@ -305,9 +312,13 @@ int __cdecl wmain( int argc, const WCHAR *argv[] )
         if (service)
         {
             CloseServiceHandle( service );
-            ret = TRUE;
+            ret = ERROR_SUCCESS;
+        }
+        else
+        {
+            ret = GetLastError();
+            WINE_ERR("failed to create service %lu\n", ret);
         }
-        else WINE_ERR("failed to create service %lu\n", GetLastError());
     }
     else if (!wcsicmp( argv[1], L"description" ))
     {
@@ -316,11 +327,18 @@ int __cdecl wmain( int argc, const WCHAR *argv[] )
         {
             SERVICE_DESCRIPTIONW sd;
             sd.lpDescription = argc > 3 ? (WCHAR *)argv[3] : NULL;
-            ret = ChangeServiceConfig2W( service, SERVICE_CONFIG_DESCRIPTION, &sd );
-            if (!ret) WINE_ERR("failed to set service description %lu\n", GetLastError());
+            if (!ChangeServiceConfig2W( service, SERVICE_CONFIG_DESCRIPTION, &sd ))
+            {
+                ret = GetLastError();
+                WINE_ERR("failed to set service description %lu\n", ret);
+            }
             CloseServiceHandle( service );
         }
-        else WINE_ERR("failed to open service %lu\n", GetLastError());
+        else
+        {
+            ret = GetLastError();
+            WINE_ERR("failed to open service %lu\n", ret);
+        }
     }
     else if (!wcsicmp( argv[1], L"failure" ))
     {
@@ -330,50 +348,80 @@ int __cdecl wmain( int argc, const WCHAR *argv[] )
             SERVICE_FAILURE_ACTIONSW sfa;
             if (parse_failure_params( argc - 3, argv + 3, &sfa ))
             {
-                ret = ChangeServiceConfig2W( service, SERVICE_CONFIG_FAILURE_ACTIONS, &sfa );
-                if (!ret) WINE_ERR("failed to set service failure actions %lu\n", GetLastError());
+                if (!ChangeServiceConfig2W( service, SERVICE_CONFIG_FAILURE_ACTIONS, &sfa ))
+                {
+                    ret = GetLastError();
+                    WINE_ERR("failed to set service failure actions %lu\n", ret);
+                }
                 free( sfa.lpsaActions );
             }
             else
                 WINE_ERR("failed to parse failure parameters\n");
             CloseServiceHandle( service );
         }
-        else WINE_ERR("failed to open service %lu\n", GetLastError());
+        else
+        {
+            ret = GetLastError();
+            WINE_ERR("failed to open service %lu\n", ret);
+        }
     }
     else if (!wcsicmp( argv[1], L"delete" ))
     {
         service = OpenServiceW( manager, argv[2], DELETE );
         if (service)
         {
-            ret = DeleteService( service );
-            if (!ret) WINE_ERR("failed to delete service %lu\n", GetLastError());
+            if (!DeleteService( service ))
+            {
+                ret = GetLastError();
+                WINE_ERR("failed to delete service %lu\n", ret);
+            }
             CloseServiceHandle( service );
         }
-        else WINE_ERR("failed to open service %lu\n", GetLastError());
+        else
+        {
+            ret = GetLastError();
+            WINE_ERR("failed to open service %lu\n", ret);
+        }
     }
     else if (!wcsicmp( argv[1], L"start" ))
     {
         service = OpenServiceW( manager, argv[2], SERVICE_START );
         if (service)
         {
-            ret = StartServiceW( service, argc - 3, argv + 3 );
-            if (!ret) WINE_ERR("failed to start service %lu\n", GetLastError());
-            else query_service( manager, argv[2] );
+            if (!StartServiceW( service, argc - 3, argv + 3 ))
+            {
+                ret = GetLastError();
+                WINE_ERR("failed to start service %lu\n", ret);
+            }
+            else
+                ret = query_service( manager, argv[2] );
             CloseServiceHandle( service );
         }
-        else WINE_ERR("failed to open service %lu\n", GetLastError());
+        else
+        {
+            ret = GetLastError();
+            WINE_ERR("failed to open service %lu\n", ret);
+        }
     }
     else if (!wcsicmp( argv[1], L"stop" ))
     {
         service = OpenServiceW( manager, argv[2], SERVICE_STOP );
         if (service)
         {
-            ret = ControlService( service, SERVICE_CONTROL_STOP, &status );
-            if (!ret) WINE_ERR("failed to stop service %lu\n", GetLastError());
-            else query_service( manager, argv[2] );
+            if (!ControlService( service, SERVICE_CONTROL_STOP, &status ))
+            {
+                ret = GetLastError();
+                WINE_ERR("failed to stop service %lu\n", ret);
+            }
+            else
+                ret = query_service( manager, argv[2] );
             CloseServiceHandle( service );
         }
-        else WINE_ERR("failed to open service %lu\n", GetLastError());
+        else
+        {
+            ret = GetLastError();
+            WINE_ERR("failed to open service %lu\n", ret);
+        }
     }
     else if (!wcsicmp( argv[1], L"query" ))
     {
@@ -382,11 +430,13 @@ int __cdecl wmain( int argc, const WCHAR *argv[] )
     else if (!wcsicmp( argv[1], L"sdset" ))
     {
         WINE_FIXME("SdSet command not supported, faking success\n");
-        ret = TRUE;
     }
     else
+    {
         WINE_FIXME("command not supported\n");
+        ret = ERROR_INVALID_COMMAND_LINE;
+    }
 
     CloseServiceHandle( manager );
-    return !ret;
+    return ret;
 }
diff --git a/programs/sc/tests/sc.c b/programs/sc/tests/sc.c
index 57046905a4b..f97c9378658 100644
--- a/programs/sc/tests/sc.c
+++ b/programs/sc/tests/sc.c
@@ -156,8 +156,6 @@ static void delete_service_(const char *file, unsigned line, const char *name, D
     strcat(command, name);
     bret = run_sc_exe_(file, line, command, &r);
     lok(bret, "run_sc_exe failed\n");
-    if (expected_status != SC_EXIT_SUCCESS && !strcmp(winetest_platform, "wine"))
-        expected_status = 1;
     todo_wine_if(broken) lok(r == expected_status, "got exit code %ld, expected %ld\n", r, expected_status);
 }
 
@@ -214,11 +212,11 @@ static void test_create_service(BOOL elevated)
     /* too few parameters */
 
     run_sc_exe("sc create", &r);
-    todo_wine check_exit_code(SC_EXIT_INVALID_COMMAND_LINE);
+    check_exit_code(SC_EXIT_INVALID_COMMAND_LINE);
     delete_test_service(FALSE, FALSE);
 
     run_sc_exe("sc create " TEST_SERVICE_NAME, &r);
-    todo_wine check_exit_code(SC_EXIT_INVALID_COMMAND_LINE);
+    check_exit_code(SC_EXIT_INVALID_COMMAND_LINE);
     delete_test_service(FALSE, FALSE);
 
     /* binpath= */
@@ -231,7 +229,7 @@ static void test_create_service(BOOL elevated)
     /* existing service */
 
     run_sc_exe("sc create " TEST_SERVICE_NAME " binpath= \"" TEST_SERVICE_BINARY "\" start= auto", &r);
-    todo_wine check_exit_code(SC_EXIT_SERVICE_EXISTS);
+    check_exit_code(SC_EXIT_SERVICE_EXISTS);
     check_test_service(SERVICE_WIN32_OWN_PROCESS, SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL, "", NULL,
                        BROKEN_DISPLAY_NAME);
     delete_test_service(TRUE, FALSE);
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/6476