diff mbox series

[Openvpn-devel,v2] Add error reporting to get_console_input_win32().

Message ID 20210618181246.30769-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,v2] Add error reporting to get_console_input_win32(). | expand

Commit Message

Gert Doering June 18, 2021, 8:12 a.m. UTC 
When the function setup fails due to invalid file handles, or because
WriteFile(err, ...) fails (due to file handle corruption elsewhere),
the function used to silently "return false"

Change this to print a M_WARN|M_ERRNO message.

Also, change the function style to early-return style (= large diff, but
most are indent changes only).

v2: fix spurious "}" that was left over from change to early-return.

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 src/openvpn/console_builtin.c | 95 ++++++++++++++++++-----------------
 1 file changed, 49 insertions(+), 46 deletions(-)

Comments

Lev Stipakov June 20, 2021, 9 p.m. UTC | #1 
Tested with MSVC. It is now easier to diagnose problems:

2021-06-20 23:57:28 us=812000 get_console_input_win32(): unexpected
error: No such device or address (errno=6)
2021-06-20 23:57:28 us=812000 ERROR: Failed retrieving username or password
2021-06-20 23:57:28 us=812000 Exiting due to fatal error

 which are to be fixed.

Acked-by: Lev Stipakov <lstipakov@gmail.com>
Gert Doering June 24, 2021, 5:30 a.m. UTC | #2 
Your patch has been applied to the master and release/2.5 branch (bugfix).

commit 8f283648d90799683d9e8f58ca4776ae1e893fd8 (master)
commit b6012d80a198b5d7e8c2f399b92a1299c660a326 (release/2.5)
Author: Gert Doering
Date:   Fri Jun 18 20:12:46 2021 +0200

     Add error reporting to get_console_input_win32().

     Signed-off-by: Gert Doering <gert@greenie.muc.de>
     Acked-by: Lev Stipakov <lstipakov@gmail.com>
     Message-Id: <20210618181246.30769-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22577.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/console_builtin.c b/src/openvpn/console_builtin.c
index 3214cb5f..9bf36347 100644
--- a/src/openvpn/console_builtin.c
+++ b/src/openvpn/console_builtin.c
@@ -75,65 +75,68 @@  get_console_input_win32(const char *prompt, const bool echo, char *input, const
     in = GetStdHandle(STD_INPUT_HANDLE);
     err = get_orig_stderr();
 
-    if (in != INVALID_HANDLE_VALUE
-        && err != INVALID_HANDLE_VALUE
-        && !win32_service_interrupt(&win32_signal)
-        && WriteFile(err, prompt, strlen(prompt), &len, NULL))
+    if (in == INVALID_HANDLE_VALUE
+        || err == INVALID_HANDLE_VALUE
+        || win32_service_interrupt(&win32_signal)
+        || !WriteFile(err, prompt, strlen(prompt), &len, NULL))
     {
-        bool is_console = (GetFileType(in) == FILE_TYPE_CHAR);
-        DWORD flags_save = 0;
-        int status = 0;
-        WCHAR *winput;
+        msg(M_WARN|M_ERRNO, "get_console_input_win32(): unexpected error");
+        return false;
+    }
 
-        if (is_console)
-        {
-            if (GetConsoleMode(in, &flags_save))
-            {
-                DWORD flags = ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT;
-                if (echo)
-                {
-                    flags |= ENABLE_ECHO_INPUT;
-                }
-                SetConsoleMode(in, flags);
-            }
-            else
-            {
-                is_console = 0;
-            }
-        }
+    bool is_console = (GetFileType(in) == FILE_TYPE_CHAR);
+    DWORD flags_save = 0;
+    int status = 0;
+    WCHAR *winput;
 
-        if (is_console)
+    if (is_console)
+    {
+        if (GetConsoleMode(in, &flags_save))
         {
-            winput = malloc(capacity * sizeof(WCHAR));
-            if (winput == NULL)
+            DWORD flags = ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT;
+            if (echo)
             {
-                return false;
+                flags |= ENABLE_ECHO_INPUT;
             }
-
-            status = ReadConsoleW(in, winput, capacity, &len, NULL);
-            WideCharToMultiByte(CP_UTF8, 0, winput, len, input, capacity, NULL, NULL);
-            free(winput);
+            SetConsoleMode(in, flags);
         }
         else
         {
-            status = ReadFile(in, input, capacity, &len, NULL);
+            is_console = 0;
         }
+    }
 
-        string_null_terminate(input, (int)len, capacity);
-        chomp(input);
-
-        if (!echo)
-        {
-            WriteFile(err, "\r\n", 2, &len, NULL);
-        }
-        if (is_console)
-        {
-            SetConsoleMode(in, flags_save);
-        }
-        if (status && !win32_service_interrupt(&win32_signal))
+    if (is_console)
+    {
+        winput = malloc(capacity * sizeof(WCHAR));
+        if (winput == NULL)
         {
-            return true;
+            return false;
         }
+
+        status = ReadConsoleW(in, winput, capacity, &len, NULL);
+        WideCharToMultiByte(CP_UTF8, 0, winput, len, input, capacity, NULL, NULL);
+        free(winput);
+    }
+    else
+    {
+        status = ReadFile(in, input, capacity, &len, NULL);
+    }
+
+    string_null_terminate(input, (int)len, capacity);
+    chomp(input);
+
+    if (!echo)
+    {
+        WriteFile(err, "\r\n", 2, &len, NULL);
+    }
+    if (is_console)
+    {
+        SetConsoleMode(in, flags_save);
+    }
+    if (status && !win32_service_interrupt(&win32_signal))
+    {
+        return true;
     }
 
     return false;