[X2Go-Commits] [libx2goclient] 03/13: src/x2goclient-{network-ssh, openssh-version}.{c, h}: split up x2goclient_network_ssh_fetch_openssh_version into same function and x2goclient_openssh_version_fetch_openssh_version which does the actual work.

git-admin at x2go.org git-admin at x2go.org
Sat Jun 27 14:42:19 CEST 2020


This is an automated email from the git hooks/post-receive script.

x2go pushed a commit to branch master
in repository libx2goclient.

commit ee1dca720dde30271b5580e7d08945f035f6097e
Author: Mihai Moldovan <ionic at ionic.de>
Date:   Sat Jun 20 09:51:16 2020 +0200

    src/x2goclient-{network-ssh,openssh-version}.{c,h}: split up x2goclient_network_ssh_fetch_openssh_version into same function and x2goclient_openssh_version_fetch_openssh_version which does the actual work.
    
    Essentially moves the most code to the other place. It really belongs
    there.
---
 src/x2goclient-network-ssh.c     | 183 +++------------------------------------
 src/x2goclient-network-ssh.h     |   2 -
 src/x2goclient-openssh-version.c | 171 ++++++++++++++++++++++++++++++++++++
 src/x2goclient-openssh-version.h |   4 +
 4 files changed, 188 insertions(+), 172 deletions(-)

diff --git a/src/x2goclient-network-ssh.c b/src/x2goclient-network-ssh.c
index e26ad58..c87e433 100644
--- a/src/x2goclient-network-ssh.c
+++ b/src/x2goclient-network-ssh.c
@@ -979,185 +979,28 @@ static gboolean x2goclient_network_ssh_parent_connect (X2GoClientNetwork *parent
   return (ret);
 }
 
-/* FIXME: this code should probably be part of X2GoClientOpenSSHVersion. */
 static gboolean x2goclient_network_ssh_fetch_openssh_version (X2GoClientNetworkSSH *self, GError **gerr) {
   gboolean ret = FALSE;
 
   g_return_val_if_fail (X2GOCLIENT_IS_NETWORK_SSH (self), ret);
   g_return_val_if_fail (((NULL == gerr) || (NULL == *gerr)), ret);
 
-  GPtrArray *ssh_cmd = g_ptr_array_new_with_free_func (&x2goclient_clear_strings);
-  g_ptr_array_add (ssh_cmd, g_strdup ("ssh"));
-  g_ptr_array_add (ssh_cmd, g_strdup ("-V"));
-  g_ptr_array_add (ssh_cmd, NULL);
-
-  {
-    gchar *tmp = NULL;
-    for (gsize i = 0; i < ssh_cmd->len; ++i) {
-      gchar *tmp_old = tmp;
-
-      if (tmp) {
-        tmp = g_strdup_printf ("%s [%s]", tmp_old, (gchar *)g_ptr_array_index (ssh_cmd, i));
-      }
-      else {
-        tmp = g_strdup_printf ("[%s]", (gchar *)g_ptr_array_index (ssh_cmd, i));
-      }
-
-      g_free (tmp_old);
-      tmp_old = NULL;
-    }
-
-    g_log (NULL, G_LOG_LEVEL_DEBUG, "Fetching OpenSSH version via: %s", tmp);
-
-    g_free (tmp);
-    tmp = NULL;
-  }
-
-  GError *ssh_err = NULL;
-  GSubprocess *ssh_proc = g_subprocess_newv ((const gchar* const*)(ssh_cmd->pdata), (G_SUBPROCESS_FLAGS_STDOUT_PIPE | G_SUBPROCESS_FLAGS_STDERR_PIPE), &ssh_err);
-
-  ret = (ssh_proc != NULL);
-
-  if (ret) {
-    g_log (NULL, G_LOG_LEVEL_DEBUG, "OpenSSH version fetching process started/executed successfully!");
-
-    if (ssh_err) {
-      g_log (NULL, G_LOG_LEVEL_DEBUG, "Successful execution, but ssh_err set? Weird, here's the message: %s", ssh_err->message);
-    }
-
-    GCancellable *ssh_proc_comm_cancel = g_cancellable_new ();
-    g_clear_error (&ssh_err);
-
-    GBytes *ssh_stdout = NULL, *ssh_stderr = NULL;
-    if (!(g_subprocess_communicate (ssh_proc, NULL, ssh_proc_comm_cancel, &ssh_stdout, &ssh_stderr, &ssh_err))) {
-      g_propagate_prefixed_error (gerr, ssh_err, "Communication with OpenSSH version fetching subprocess failed: ");
-    }
-    else {
-      gsize ssh_stdout_size = 0, ssh_stderr_size = 0;
-      const gchar *ssh_stdout_str = g_bytes_get_data (ssh_stdout, &ssh_stdout_size),
-                  *ssh_stderr_str = g_bytes_get_data (ssh_stderr, &ssh_stderr_size);
-      int ssh_stdout_size_sanitized = 0, ssh_stderr_size_sanitized = 0;
-      gchar *ssh_stdout_str_sanitized = 0, *ssh_stderr_str_sanitized = NULL;
-
-      /*
-       * FIXME: something about this section looks odd, revisit.
-       *        Also, we should probably drop everything but the first line
-       *        early on.
-       */
-      /* Sanity check on output size. */
-      if (INT_MAX < ssh_stdout_size) {
-        g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH returned more than %d bytes on stdout, this is unusual and will be truncated.", INT_MAX);
+  X2GoClientOpenSSHVersion *version = x2goclient_openssh_version_fetch_openssh_version (gerr);
 
-        ssh_stdout_size_sanitized = INT_MAX;
-      }
-      else {
-        ssh_stdout_size_sanitized = ssh_stdout_size;
-      }
-
-      if (ssh_stdout_str) {
-        /*
-         * Do NOT use g_strndup () here.
-         *
-         * It might sound exactly like what we want, but really isn't.
-         *
-         * The issue is that g_strndup () will always allocate an n-bytes-sized
-         * buffer and optionally pad the string with NULL bytes.
-         *
-         * That's not really a problem if the string actually is to be
-         * truncated, since in that case the result will be smaller than the
-         * original string anyway, but a problem if the size is huge, but the
-         * string small.
-         *
-         * In the latter case, we don't want to create a useless 2 GiB (bounded
-         * by INT_MAX) string for a string that is actually quite small.
-         *
-         * Interestingly, POSIX describes strndup as allocating memory "as if
-         * by using malloc ()", but doesn't mention the actual resulting size.
-         * In the informal section, buffer sizes are described as either
-         * (size + 1) or ((strnlen (s, size)) + 1), which, again, could lead
-         * to the same problem.
-         */
-        ssh_stdout_str_sanitized = g_strdup_printf ("%.*s", ssh_stdout_size_sanitized, ssh_stdout_str);
-      }
-      else {
-        ssh_stderr_size_sanitized = ssh_stderr_size;
-      }
-
-      if (INT_MAX < ssh_stderr_size) {
-        g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH returned more than %d bytes on stderr, this is unusual and will be truncated.", INT_MAX);
-
-        ssh_stderr_size_sanitized = INT_MAX;
-      }
-
-      if (ssh_stderr_str) {
-        ssh_stderr_str_sanitized = g_strdup_printf ("%.*s", ssh_stderr_size_sanitized, ssh_stderr_str);
-      }
-
-      g_log (NULL, G_LOG_LEVEL_DEBUG, "Stdout:\n>>>%.*s<<<\nStderr:\n>>>%.*s<<<", ssh_stdout_size_sanitized, ssh_stdout_str_sanitized, ssh_stderr_size_sanitized, ssh_stderr_str_sanitized);
-
-      if (ssh_stdout_size_sanitized) {
-        g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH version command wrote data on stdout, this is unexpected and will be ignored.\nData: %*.s", ssh_stdout_size_sanitized, ssh_stdout_str_sanitized);
-      }
-
-      if (!(ssh_stderr_size_sanitized)) {
-        g_set_error_literal (gerr, X2GOCLIENT_NETWORK_SSH_ERROR, X2GOCLIENT_NETWORK_SSH_ERROR_OPENSSH_VERSION_NO_STDERR, "OpenSSH version command wrote nothing to stderr, this is unexpected. Can't parse version string.");
-        ret = FALSE;
-      }
-      else {
-        X2GoClientOpenSSHVersion *version = x2goclient_openssh_version_new ();
-
-        if (!(version)) {
-          g_set_error_literal (gerr, X2GOCLIENT_NETWORK_SSH_ERROR, X2GOCLIENT_NETWORK_SSH_ERROR_OPENSSH_VERSION_ALLOCATE, "Unable to allocate buffe for OpenSSH version structure - memory issues?");
-          ret = FALSE;
-        }
-        else {
-          /* gerr is supposed to be empty at that point. */
-          g_assert ((NULL == gerr) || (NULL == *gerr));
-
-          ret = x2goclient_openssh_version_parse (version, ssh_stderr_str_sanitized, gerr);
-
-          if (ret) {
-            /*
-             * Everything went well, copy to the property.
-             *
-             * ... unfortunately, the actual property is read-only (which
-             * totally makes sense because we only want this class to handle
-             * it), so we'll have to duplicate a bit of code and can't use the
-             * GObject property setter directly.
-             */
-            x2goclient_openssh_version_free (self->openssh_version);
-            self->openssh_version = version;
-          }
-          else {
-            /* Get rid of the version struct. It's bogus anyway. */
-            x2goclient_openssh_version_free (version);
-          }
-
-          version = NULL;
-        }
-      }
-
-      g_free (ssh_stdout_str_sanitized);
-      g_free (ssh_stderr_str_sanitized);
-
-      g_bytes_unref (ssh_stdout);
-      g_bytes_unref (ssh_stderr);
-    }
+  if (version) {
+    /*
+     * Everything went well, copy to the property.
+     *
+     * ... unfortunately, the actual property is read-only (which
+     * totally makes sense because we only want this class to handle
+     * it), so we'll have to duplicate a bit of code and can't use the
+     * GObject property setter directly.
+     */
+    x2goclient_openssh_version_free (self->openssh_version);
+    self->openssh_version = version;
 
-    g_clear_object (&ssh_proc_comm_cancel);
-    ssh_proc_comm_cancel = NULL;
-  }
-  else {
-    g_propagate_prefixed_error (gerr, ssh_err, "OpenSSH vesion fetching process didn't execute/start successfully! Error: ");
+    ret = TRUE;
   }
 
-  g_clear_error (&ssh_err);
-
-  g_clear_object (&ssh_proc);
-  ssh_proc = NULL;
-
-  g_ptr_array_unref (ssh_cmd);
-  ssh_cmd = NULL;
-
   return (ret);
 }
diff --git a/src/x2goclient-network-ssh.h b/src/x2goclient-network-ssh.h
index b85f49b..80cc10f 100644
--- a/src/x2goclient-network-ssh.h
+++ b/src/x2goclient-network-ssh.h
@@ -67,8 +67,6 @@ enum {
   X2GOCLIENT_NETWORK_SSH_ERROR_CONNECT_NATIVE_FETCH,
   X2GOCLIENT_NETWORK_SSH_ERROR_CONNECT_SOCK_ADDR_UNKNOWN,
   X2GOCLIENT_NETWORK_SSH_ERROR_CONNECT_OPTIONS_INVALID,
-  X2GOCLIENT_NETWORK_SSH_ERROR_OPENSSH_VERSION_NO_STDERR,
-  X2GOCLIENT_NETWORK_SSH_ERROR_OPENSSH_VERSION_ALLOCATE,
 };
 
 
diff --git a/src/x2goclient-openssh-version.c b/src/x2goclient-openssh-version.c
index 440e533..fbdabfd 100644
--- a/src/x2goclient-openssh-version.c
+++ b/src/x2goclient-openssh-version.c
@@ -26,6 +26,7 @@
 
 #include <glib.h>
 #include <glib/gprintf.h>
+#include <gio/gio.h>
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -366,3 +367,173 @@ _Bool x2goclient_openssh_version_parse (X2GoClientOpenSSHVersion *openssh_versio
 
   return (ret);
 }
+
+X2GoClientOpenSSHVersion* x2goclient_openssh_version_fetch_openssh_version (GError **gerr) {
+  X2GoClientOpenSSHVersion *ret = NULL;
+  gboolean cont = FALSE;
+
+  g_return_val_if_fail (((NULL == gerr) || (NULL == *gerr)), ret);
+
+  GPtrArray *ssh_cmd = g_ptr_array_new_with_free_func (&x2goclient_clear_strings);
+  g_ptr_array_add (ssh_cmd, g_strdup ("ssh"));
+  g_ptr_array_add (ssh_cmd, g_strdup ("-V"));
+  g_ptr_array_add (ssh_cmd, NULL);
+
+  {
+    gchar *tmp = NULL;
+    for (gsize i = 0; i < ssh_cmd->len; ++i) {
+      gchar *tmp_old = tmp;
+
+      if (tmp) {
+        tmp = g_strdup_printf ("%s [%s]", tmp_old, (gchar *)g_ptr_array_index (ssh_cmd, i));
+      }
+      else {
+        tmp = g_strdup_printf ("[%s]", (gchar *)g_ptr_array_index (ssh_cmd, i));
+      }
+
+      g_free (tmp_old);
+      tmp_old = NULL;
+    }
+
+    g_log (NULL, G_LOG_LEVEL_DEBUG, "Fetching OpenSSH version via: %s", tmp);
+
+    g_free (tmp);
+    tmp = NULL;
+  }
+
+  GError *ssh_err = NULL;
+  GSubprocess *ssh_proc = g_subprocess_newv ((const gchar* const*)(ssh_cmd->pdata), (G_SUBPROCESS_FLAGS_STDOUT_PIPE | G_SUBPROCESS_FLAGS_STDERR_PIPE), &ssh_err);
+
+  cont = (ssh_proc != NULL);
+
+  if (cont) {
+    g_log (NULL, G_LOG_LEVEL_DEBUG, "OpenSSH version fetching process started/executed successfully!");
+
+    if (ssh_err) {
+      g_log (NULL, G_LOG_LEVEL_DEBUG, "Successful execution, but ssh_err set? Weird, here's the message: %s", ssh_err->message);
+    }
+
+    GCancellable *ssh_proc_comm_cancel = g_cancellable_new ();
+    g_clear_error (&ssh_err);
+
+    GBytes *ssh_stdout = NULL, *ssh_stderr = NULL;
+    if (!(g_subprocess_communicate (ssh_proc, NULL, ssh_proc_comm_cancel, &ssh_stdout, &ssh_stderr, &ssh_err))) {
+      g_propagate_prefixed_error (gerr, ssh_err, "Communication with OpenSSH version fetching subprocess failed: ");
+    }
+    else {
+      gsize ssh_stdout_size = 0, ssh_stderr_size = 0;
+      const gchar *ssh_stdout_str = g_bytes_get_data (ssh_stdout, &ssh_stdout_size),
+                  *ssh_stderr_str = g_bytes_get_data (ssh_stderr, &ssh_stderr_size);
+      int ssh_stdout_size_sanitized = 0, ssh_stderr_size_sanitized = 0;
+      gchar *ssh_stdout_str_sanitized = 0, *ssh_stderr_str_sanitized = NULL;
+
+      /*
+       * FIXME: something about this section looks odd, revisit.
+       *        Also, we should probably drop everything but the first line
+       *        early on.
+       */
+      /* Sanity check on output size. */
+      if (INT_MAX < ssh_stdout_size) {
+        g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH returned more than %d bytes on stdout, this is unusual and will be truncated.", INT_MAX);
+
+        ssh_stdout_size_sanitized = INT_MAX;
+      }
+      else {
+        ssh_stdout_size_sanitized = ssh_stdout_size;
+      }
+
+      if (ssh_stdout_str) {
+        /*
+         * Do NOT use g_strndup () here.
+         *
+         * It might sound exactly like what we want, but really isn't.
+         *
+         * The issue is that g_strndup () will always allocate an n-bytes-sized
+         * buffer and optionally pad the string with NULL bytes.
+         *
+         * That's not really a problem if the string actually is to be
+         * truncated, since in that case the result will be smaller than the
+         * original string anyway, but a problem if the size is huge, but the
+         * string small.
+         *
+         * In the latter case, we don't want to create a useless 2 GiB (bounded
+         * by INT_MAX) string for a string that is actually quite small.
+         *
+         * Interestingly, POSIX describes strndup as allocating memory "as if
+         * by using malloc ()", but doesn't mention the actual resulting size.
+         * In the informal section, buffer sizes are described as either
+         * (size + 1) or ((strnlen (s, size)) + 1), which, again, could lead
+         * to the same problem.
+         */
+        ssh_stdout_str_sanitized = g_strdup_printf ("%.*s", ssh_stdout_size_sanitized, ssh_stdout_str);
+      }
+      else {
+        ssh_stderr_size_sanitized = ssh_stderr_size;
+      }
+
+      if (INT_MAX < ssh_stderr_size) {
+        g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH returned more than %d bytes on stderr, this is unusual and will be truncated.", INT_MAX);
+
+        ssh_stderr_size_sanitized = INT_MAX;
+      }
+
+      if (ssh_stderr_str) {
+        ssh_stderr_str_sanitized = g_strdup_printf ("%.*s", ssh_stderr_size_sanitized, ssh_stderr_str);
+      }
+
+      g_log (NULL, G_LOG_LEVEL_DEBUG, "Stdout:\n>>>%.*s<<<\nStderr:\n>>>%.*s<<<", ssh_stdout_size_sanitized, ssh_stdout_str_sanitized, ssh_stderr_size_sanitized, ssh_stderr_str_sanitized);
+
+      if (ssh_stdout_size_sanitized) {
+        g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH version command wrote data on stdout, this is unexpected and will be ignored.\nData: %*.s", ssh_stdout_size_sanitized, ssh_stdout_str_sanitized);
+      }
+
+      if (!(ssh_stderr_size_sanitized)) {
+        g_set_error_literal (gerr, X2GOCLIENT_OPENSSH_VERSION_ERROR, X2GOCLIENT_OPENSSH_VERSION_ERROR_OPENSSH_VERSION_NO_STDERR, "OpenSSH version command wrote nothing to stderr, this is unexpected. Can't parse version string.");
+        cont = FALSE;
+      }
+      else {
+        ret = x2goclient_openssh_version_new ();
+
+        if (!(ret)) {
+          g_set_error_literal (gerr, X2GOCLIENT_OPENSSH_VERSION_ERROR, X2GOCLIENT_OPENSSH_VERSION_ERROR_OPENSSH_VERSION_ALLOCATE, "Unable to allocate buffer for OpenSSH version structure - memory issues?");
+          cont = FALSE;
+        }
+        else {
+          /* gerr is supposed to be empty at that point. */
+          g_assert ((NULL == gerr) || (NULL == *gerr));
+
+          cont = x2goclient_openssh_version_parse (ret, ssh_stderr_str_sanitized, gerr);
+
+          if (!(cont)) {
+            /* Get rid of the version struct. It's bogus anyway. */
+            x2goclient_openssh_version_free (ret);
+
+            ret = NULL;
+          }
+        }
+      }
+
+      g_free (ssh_stdout_str_sanitized);
+      g_free (ssh_stderr_str_sanitized);
+
+      g_bytes_unref (ssh_stdout);
+      g_bytes_unref (ssh_stderr);
+    }
+
+    g_clear_object (&ssh_proc_comm_cancel);
+    ssh_proc_comm_cancel = NULL;
+  }
+  else {
+    g_propagate_prefixed_error (gerr, ssh_err, "OpenSSH vesion fetching process didn't execute/start successfully! Error: ");
+  }
+
+  g_clear_error (&ssh_err);
+
+  g_clear_object (&ssh_proc);
+  ssh_proc = NULL;
+
+  g_ptr_array_unref (ssh_cmd);
+  ssh_cmd = NULL;
+
+  return (ret);
+}
diff --git a/src/x2goclient-openssh-version.h b/src/x2goclient-openssh-version.h
index 0df8ada..42b1273 100644
--- a/src/x2goclient-openssh-version.h
+++ b/src/x2goclient-openssh-version.h
@@ -75,10 +75,14 @@ enum {
   X2GOCLIENT_OPENSSH_VERSION_ERROR_PATCH_OVERFLOW,
   X2GOCLIENT_OPENSSH_VERSION_ERROR_PATCH_TOO_BIG,
   X2GOCLIENT_OPENSSH_VERSION_ERROR_PATCH_NEGATIVE,
+  X2GOCLIENT_OPENSSH_VERSION_ERROR_OPENSSH_VERSION_NO_STDERR,
+  X2GOCLIENT_OPENSSH_VERSION_ERROR_OPENSSH_VERSION_ALLOCATE,
 };
 
 
 _Bool x2goclient_openssh_version_parse (X2GoClientOpenSSHVersion *openssh_version, const gchar * const version_string, GError **gerr);
+X2GoClientOpenSSHVersion* x2goclient_openssh_version_fetch_openssh_version (GError **gerr);
+
 
 G_END_DECLS
 

--
Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/libx2goclient.git


More information about the x2go-commits mailing list