diff mbox series

[Openvpn-devel] Refactor OpenVPNService state detection code

Message ID 20190224181544.17232-1-simon@rozman.si
State Accepted
Headers
Series [Openvpn-devel] Refactor OpenVPNService state detection code |

Commit Message

Simon Rozman Feb. 24, 2019, 7:15 a.m. UTC 
  The code was standardized to avoid "E1072: a declaration cannot have a
label" warning of Visual Studio 2017 IntelliSense.

Furthermore, a comment explaining what `dwStartType <=
SERVICE_AUTO_START` condition is about.

This patch follows Gert's recommendations from [openvpn-devel].

Signed-off-by: Simon Rozman <simon@rozman.si>
Message-ID: <201901181944.x0IJiGuV003728@chekov.greenie.muc.de>
---
 src/openvpnmsica/openvpnmsica.c | 59 +++++++++++++++++----------------
 1 file changed, 31 insertions(+), 28 deletions(-)

Comments

Gert Doering Feb. 28, 2019, 4:26 a.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

(Basically just reworking if/goto into if/else, plus more verbose comments.
"Git show -w --color-moved=zebra" helps in seeing "nothing really changed")

Your patch has been applied to the master branch.

commit 197f8076e43534f3dfe50e3c638143c38f4bad7f
Author: Simon Rozman
Date:   Sun Feb 24 19:15:44 2019 +0100

     Refactor OpenVPNService state detection code

     Signed-off-by: Simon Rozman <simon@rozman.si>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20190224181544.17232-1-simon@rozman.si>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18233.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index 00ed2765..2477e81a 100644
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -365,40 +365,42 @@  openvpnmsica_set_openvpnserv_state(_In_ MSIHANDLE hInstall)
     /* Query service status. */
     SERVICE_STATUS_PROCESS ssp;
     DWORD dwBufSize;
-    if (!QueryServiceStatusEx(hService, SC_STATUS_PROCESS_INFO, (LPBYTE)&ssp, sizeof(ssp), &dwBufSize))
+    if (QueryServiceStatusEx(hService, SC_STATUS_PROCESS_INFO, (LPBYTE)&ssp, sizeof(ssp), &dwBufSize))
     {
-        uiResult = GetLastError();
-        msg(M_NONFATAL | M_ERRNO, "%s: QueryServiceStatusEx(\"OpenVPNService\") failed", __FUNCTION__);
-        goto finish_QueryServiceStatusEx;
-    }
-
-    switch (ssp.dwCurrentState)
-    {
-        case SERVICE_START_PENDING:
-        case SERVICE_RUNNING:
-        case SERVICE_STOP_PENDING:
-        case SERVICE_PAUSE_PENDING:
-        case SERVICE_PAUSED:
-        case SERVICE_CONTINUE_PENDING:
+        switch (ssp.dwCurrentState)
         {
-            /* Set OPENVPNSERVICE property to service PID. */
-            TCHAR szPID[10 /*MAXDWORD in decimal*/ + 1 /*terminator*/];
-            _stprintf_s(
-                szPID, _countof(szPID),
-                TEXT("%u"),
-                ssp.dwProcessId);
-
-            uiResult = MsiSetProperty(hInstall, TEXT("OPENVPNSERVICE"), szPID);
-            if (uiResult != ERROR_SUCCESS)
+            case SERVICE_START_PENDING:
+            case SERVICE_RUNNING:
+            case SERVICE_STOP_PENDING:
+            case SERVICE_PAUSE_PENDING:
+            case SERVICE_PAUSED:
+            case SERVICE_CONTINUE_PENDING:
             {
-                SetLastError(uiResult); /* MSDN does not mention MsiSetProperty() to set GetLastError(). But we do have an error code. Set last error manually. */
-                msg(M_NONFATAL | M_ERRNO, "%s: MsiSetProperty(\"OPENVPNSERVICE\") failed", __FUNCTION__);
+                /* Service is started (kind of). Set OPENVPNSERVICE property to service PID. */
+                TCHAR szPID[10 /*MAXDWORD in decimal*/ + 1 /*terminator*/];
+                _stprintf_s(
+                    szPID, _countof(szPID),
+                    TEXT("%u"),
+                    ssp.dwProcessId);
+
+                uiResult = MsiSetProperty(hInstall, TEXT("OPENVPNSERVICE"), szPID);
+                if (uiResult != ERROR_SUCCESS)
+                {
+                    SetLastError(uiResult); /* MSDN does not mention MsiSetProperty() to set GetLastError(). But we do have an error code. Set last error manually. */
+                    msg(M_NONFATAL | M_ERRNO, "%s: MsiSetProperty(\"OPENVPNSERVICE\") failed", __FUNCTION__);
+                }
+
+                /* We know user is using the service. Skip auto-start setting check. */
+                goto cleanup_OpenService;
             }
-            goto cleanup_OpenService;
+            break;
         }
-        break;
     }
-finish_QueryServiceStatusEx:;
+    else
+    {
+        uiResult = GetLastError();
+        msg(M_NONFATAL | M_ERRNO, "%s: QueryServiceStatusEx(\"OpenVPNService\") failed", __FUNCTION__);
+    }
 
     /* Service is not started. Is it set to auto-start? */
     /* MSDN describes the maximum buffer size for QueryServiceConfig() to be 8kB. */
@@ -415,6 +417,7 @@  finish_QueryServiceStatusEx:;
 
     if (pQsc->dwStartType <= SERVICE_AUTO_START)
     {
+        /* Service is set to auto-start. Set OPENVPNSERVICE property to its path. */
         uiResult = MsiSetProperty(hInstall, TEXT("OPENVPNSERVICE"), pQsc->lpBinaryPathName);
         if (uiResult != ERROR_SUCCESS)
         {