Re: [Openvpn-devel] [PATCH 48/52] cleanup: move console related function into its own module

[Gert Doering]

Hi,

On Thu, Mar 08, 2012 at 03:23:50PM +0200, Samuli Seppänen wrote:
> Currently many files (42) include misc.h, which is a lot. Also, misc.c
> is among the biggest files in the codebase[1], so splitting it into
> smaller files with good names and well-defined functionality makes sense:
> 
> "Oh, this include console.h, it must be using some console-related
> functions"
> 
> Feature-vise I give this one an ACK. Somebody else might want to look at
> the actual changes.

ACK on the actual changes.

I verified that the code only moves, and is not changed.

Three exceptions from "code only moves":
  - getpass() disappears, because it's dead code anyway
  - the declaration of cmd[256] from get_console_input_systemd() 
disappears, because it's unused 
  - a few 'static' declarations need to go away

but these are fine, so ACK.

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


pgpXeq56Sz2B8.pgp
Description: PGP signature

Re: [Openvpn-devel] [PATCH 48/52] cleanup: move console related function into its own module

[Samuli Seppänen]

Currently many files (42) include misc.h, which is a lot. Also, misc.c
is among the biggest files in the codebase[1], so splitting it into
smaller files with good names and well-defined functionality makes sense:

"Oh, this include console.h, it must be using some console-related
functions"

Feature-vise I give this one an ACK. Somebody else might want to look at
the actual changes.

-- 
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock

[1] For you own amusement, you can try something like this:

for i in `ls *.c`;do echo -n $i": ";cat $i|wc --lines;done|less
> Signed-off-by: Alon Bar-Lev 
> ---
>  src/openvpn/Makefile.am|1 +
>  src/openvpn/console.c  |  238 
> 
>  src/openvpn/console.h  |   33 ++
>  src/openvpn/misc.c |  125 +---
>  src/openvpn/misc.h |3 +-
>  src/openvpn/openvpn.vcproj |8 ++
>  src/openvpn/pkcs11.c   |1 +
>  src/openvpn/win32.c|   87 +
>  src/openvpn/win32.h|5 +-
>  9 files changed, 286 insertions(+), 215 deletions(-)
>  create mode 100644 src/openvpn/console.c
>  create mode 100644 src/openvpn/console.h
>
> diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
> index e170380..333eebc 100644
> --- a/src/openvpn/Makefile.am
> +++ b/src/openvpn/Makefile.am
> @@ -58,6 +58,7 @@ openvpn_SOURCES = \
> mbuf.c mbuf.h \
> memdbg.h \
> misc.c misc.h \
> +   console.c console.h \
> mroute.c mroute.h \
> mss.c mss.h \
> mstats.c mstats.h \
> diff --git a/src/openvpn/console.c b/src/openvpn/console.c
> new file mode 100644
> index 000..2464e7e
> --- /dev/null
> +++ b/src/openvpn/console.c
> @@ -0,0 +1,238 @@
> +/*
> + *  OpenVPN -- An application to securely tunnel IP networks
> + * over a single UDP port, with support for SSL/TLS-based
> + * session authentication and key exchange,
> + * packet encryption, packet authentication, and
> + * packet compression.
> + *
> + *  Copyright (C) 2002-2010 OpenVPN Technologies, Inc. 
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2
> + *  as published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program (see the file COPYING included with this
> + *  distribution); if not, write to the Free Software Foundation, Inc.,
> + *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#elif defined(_MSC_VER)
> +#include "config-msvc.h"
> +#endif
> +
> +#include "syshead.h"
> +#include "console.h"
> +#include "error.h"
> +#include "buffer.h"
> +#include "misc.h"
> +
> +#ifdef WIN32
> +
> +#include "win32.h"
> +
> +/*
> + * Get input from console.
> + *
> + * Return false on input error, or if service
> + * exit event is signaled.
> + */
> +
> +static bool
> +get_console_input_win32 (const char *prompt, const bool echo, char *input, 
> const int capacity)
> +{
> +  HANDLE in = INVALID_HANDLE_VALUE;
> +  HANDLE err = INVALID_HANDLE_VALUE;
> +  DWORD len = 0;
> +
> +  ASSERT (prompt);
> +  ASSERT (input);
> +  ASSERT (capacity > 0);
> +
> +  input[0] = '\0';
> +
> +  in = GetStdHandle (STD_INPUT_HANDLE);
> +  err = get_orig_stderr ();
> +
> +  if (in != INVALID_HANDLE_VALUE
> +  && err != INVALID_HANDLE_VALUE
> +  && !win32_service_interrupt (_signal)
> +  && WriteFile (err, prompt, strlen (prompt), , NULL))
> +{
> +  bool is_console = (GetFileType (in) == FILE_TYPE_CHAR);
> +  DWORD flags_save = 0;
> +  int status = 0;
> +  WCHAR *winput;
> +
> +  if (is_console)
> +   {
> + if (GetConsoleMode (in, _save))
> +   {
> + DWORD flags = ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT;
> + if (echo)
> +   flags |= ENABLE_ECHO_INPUT;
> + SetConsoleMode (in, flags);
> +   }
> + else
> +   is_console = 0;
> +   }
> +
> +  if (is_console)
> +{
> +  winput = malloc (capacity * sizeof (WCHAR));
> +  if (winput == NULL)
> +return false;
> +
> +  status = ReadConsoleW (in, winput, capacity, , NULL);
> +  WideCharToMultiByte (CP_UTF8, 0, winput, len, input, capacity, 
> NULL, NULL);
> +  free (winput);
> +}
> +  else
> +status = ReadFile (in, input, capacity, , NULL);
> +
> +  string_null_terminate (input, (int)len, 

[Openvpn-devel] [PATCH 48/52] cleanup: move console related function into its own module

[Alon Bar-Lev]

Signed-off-by: Alon Bar-Lev 
---
 src/openvpn/Makefile.am|1 +
 src/openvpn/console.c  |  238 
 src/openvpn/console.h  |   33 ++
 src/openvpn/misc.c |  125 +---
 src/openvpn/misc.h |3 +-
 src/openvpn/openvpn.vcproj |8 ++
 src/openvpn/pkcs11.c   |1 +
 src/openvpn/win32.c|   87 +
 src/openvpn/win32.h|5 +-
 9 files changed, 286 insertions(+), 215 deletions(-)
 create mode 100644 src/openvpn/console.c
 create mode 100644 src/openvpn/console.h

diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index e170380..333eebc 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -58,6 +58,7 @@ openvpn_SOURCES = \
mbuf.c mbuf.h \
memdbg.h \
misc.c misc.h \
+   console.c console.h \
mroute.c mroute.h \
mss.c mss.h \
mstats.c mstats.h \
diff --git a/src/openvpn/console.c b/src/openvpn/console.c
new file mode 100644
index 000..2464e7e
--- /dev/null
+++ b/src/openvpn/console.c
@@ -0,0 +1,238 @@
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ * over a single UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ *  Copyright (C) 2002-2010 OpenVPN Technologies, Inc. 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program (see the file COPYING included with this
+ *  distribution); if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#elif defined(_MSC_VER)
+#include "config-msvc.h"
+#endif
+
+#include "syshead.h"
+#include "console.h"
+#include "error.h"
+#include "buffer.h"
+#include "misc.h"
+
+#ifdef WIN32
+
+#include "win32.h"
+
+/*
+ * Get input from console.
+ *
+ * Return false on input error, or if service
+ * exit event is signaled.
+ */
+
+static bool
+get_console_input_win32 (const char *prompt, const bool echo, char *input, 
const int capacity)
+{
+  HANDLE in = INVALID_HANDLE_VALUE;
+  HANDLE err = INVALID_HANDLE_VALUE;
+  DWORD len = 0;
+
+  ASSERT (prompt);
+  ASSERT (input);
+  ASSERT (capacity > 0);
+
+  input[0] = '\0';
+
+  in = GetStdHandle (STD_INPUT_HANDLE);
+  err = get_orig_stderr ();
+
+  if (in != INVALID_HANDLE_VALUE
+  && err != INVALID_HANDLE_VALUE
+  && !win32_service_interrupt (_signal)
+  && WriteFile (err, prompt, strlen (prompt), , NULL))
+{
+  bool is_console = (GetFileType (in) == FILE_TYPE_CHAR);
+  DWORD flags_save = 0;
+  int status = 0;
+  WCHAR *winput;
+
+  if (is_console)
+   {
+ if (GetConsoleMode (in, _save))
+   {
+ DWORD flags = ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT;
+ if (echo)
+   flags |= ENABLE_ECHO_INPUT;
+ SetConsoleMode (in, flags);
+   }
+ else
+   is_console = 0;
+   }
+
+  if (is_console)
+{
+  winput = malloc (capacity * sizeof (WCHAR));
+  if (winput == NULL)
+return false;
+
+  status = ReadConsoleW (in, winput, capacity, , NULL);
+  WideCharToMultiByte (CP_UTF8, 0, winput, len, input, capacity, NULL, 
NULL);
+  free (winput);
+}
+  else
+status = ReadFile (in, input, capacity, , NULL);
+
+  string_null_terminate (input, (int)len, capacity);
+  chomp (input);
+
+  if (!echo)
+   WriteFile (err, "\r\n", 2, , NULL);
+  if (is_console)
+   SetConsoleMode (in, flags_save);
+  if (status && !win32_service_interrupt (_signal))
+   return true;
+}
+
+  return false;
+}
+
+#endif
+
+#ifdef HAVE_GETPASS
+
+static FILE *
+open_tty (const bool write)
+{
+  FILE *ret;
+  ret = fopen ("/dev/tty", write ? "w" : "r");
+  if (!ret)
+ret = write ? stderr : stdin;
+  return ret;
+}
+
+static void
+close_tty (FILE *fp)
+{
+  if (fp != stderr && fp != stdin)
+fclose (fp);
+}
+
+#endif
+
+#ifdef ENABLE_SYSTEMD
+
+/*
+ * is systemd running
+ */
+
+static bool
+check_systemd_running ()
+{
+  struct stat a, b;
+
+  /* We simply test whether the systemd cgroup hierarchy is
+   * mounted */
+
+  return (lstat("/sys/fs/cgroup", ) == 0)
+ && (lstat("/sys/fs/cgroup/systemd",