[X2Go-Commits] [libx2goclient] 03/09: src/x2goclient-openssh-version.{c, h}: rework version parsing algorithm.

git-admin at x2go.org git-admin at x2go.org
Wed May 13 16:41:35 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 1496e50dc0a6acdeb9433c758fc07b8c34eef4a1
Author: Mihai Moldovan <ionic at ionic.de>
Date:   Mon Apr 27 19:42:02 2020 +0200

    src/x2goclient-openssh-version.{c,h}: rework version parsing algorithm.
    
    Instead of copy-pasting the same number version parsing code three times
    in a row, use a loop and some mighty processor magic to get the job
    done.
    
    That'll increase code size instead of decrease it (ironically), but it
    also looks a bit cleaner and encapsulates the version parsing part
    nicely.
    
    Additionally, this also fixes a few parsing bugs.
---
 src/x2goclient-openssh-version.c | 344 ++++++++++++++++++++++-----------------
 src/x2goclient-openssh-version.h |   3 +
 2 files changed, 198 insertions(+), 149 deletions(-)

diff --git a/src/x2goclient-openssh-version.c b/src/x2goclient-openssh-version.c
index 28b50c3..5258fce 100644
--- a/src/x2goclient-openssh-version.c
+++ b/src/x2goclient-openssh-version.c
@@ -22,6 +22,8 @@
    Boston, MA  02110-1301, USA.
  */
 
+#include <string.h>
+
 #include <glib.h>
 #include <glib/gprintf.h>
 
@@ -53,7 +55,65 @@ G_DEFINE_BOXED_TYPE (X2GoClientOpenSSHVersion, x2goclient_openssh_version, &x2go
 
 #define ERROR_QUARK EXPAND(X2GOCLIENT_OPENSSH_VERSION_ERROR)
 #define GENERATE_ERROR_(preamble, component, error) preamble ## _ ## component ## _ ## error
-#define GENERATE_ERROR(component, error) GENERATE_ERROR_(X2GOCLIENT_OPENSSH_VERSION_ERROR, component, error)
+#define GENERATE_ERROR(component, error) GENERATE_ERROR_ (X2GOCLIENT_OPENSSH_VERSION_ERROR, component, error)
+#define GENERATE_ERROR_MSG_(component, msg) #component " " msg
+#define GENERATE_ERROR_MSG(out_var, idx_var, msg) \
+  do {\
+    if (0 == idx_var) {\
+      out_var = GENERATE_ERROR_MSG_ ("Major", msg);\
+    }\
+    else if (1 == idx_var) {\
+      out_var = GENERATE_ERROR_MSG_ ("Minor", msg);\
+    }\
+    else {\
+      out_var = GENERATE_ERROR_MSG_ ("Patch", msg);\
+    }\
+  } while (0)
+#define GENERATE_ERROR_CODE(out_var, idx_var, error) \
+  do {\
+    if (0 == idx_var) {\
+      out_var = GENERATE_ERROR (MAJOR, error);\
+    }\
+    else if (1 == idx_var) {\
+      out_var = GENERATE_ERROR (MINOR, error);\
+    }\
+    else {\
+      out_var = GENERATE_ERROR (PATCH, error);\
+    }\
+  } while (0)
+#define SET_VER_COMPONENT(out_struct, idx_var, value) \
+  do {\
+    if (0 == idx_var) {\
+      out_struct->major = value;\
+    }\
+    else if (1 == idx_var) {\
+      out_struct->minor = value;\
+    }\
+    else {\
+      out_struct->patch = value;\
+    }\
+  } while (0)
+#define CHECK_VER_COMPONENT(out_struct, idx_var, err) \
+  do {\
+    if (0 == idx_var) {\
+      if (0 > out_struct->major) {\
+        err = TRUE;\
+        break;\
+      }\
+    }\
+    else if (1 == idx_var) {\
+      if (0 > out_struct->minor) {\
+        err = TRUE;\
+        break;\
+      }\
+    }\
+    else {\
+      if (0 > out_struct->patch) {\
+        err = TRUE;\
+        break;\
+      }\
+    }\
+  } while (0)
 
 void x2goclient_openssh_version_parse (X2GoClientOpenSSHVersion *openssh_version, const gchar *version_string, GError **gerr) {
   /* Basic sanity checks on struct and string. */
@@ -85,186 +145,172 @@ void x2goclient_openssh_version_parse (X2GoClientOpenSSHVersion *openssh_version
     return;
   }
 
-  /* Skip preamble, scan for major version number separator (dot). */
+  /* Skip preamble. */
   tmp += 8;
-  const gchar *dot_search = g_strstr_len (tmp, -1, ".");
-  if (!(dot_search)) {
-    g_set_error_literal (gerr, ERROR_QUARK, X2GOCLIENT_OPENSSH_VERSION_ERROR_NO_MAJOR_SEPARATOR, "Version string does not contain major version number separator.");
-
-    x2goclient_openssh_version_free (struct_work_copy);
-    struct_work_copy = NULL;
 
-    return;
-  }
+  _Bool vers_err = FALSE;
 
-  g_assert (tmp < dot_search);
-
-  /* Sanity check on major version number - must be numeric/all digits. */
-  const gchar *int_search = NULL;
-  for (int_search = tmp; int_search < dot_search; ++int_search) {
-    if (!(g_ascii_isdigit (*int_search))) {
-      g_set_error_literal (gerr, ERROR_QUARK, X2GOCLIENT_OPENSSH_VERSION_ERROR_MAJOR_NOT_NUMERIC, "Major version number is not numeric.");
+  /*
+   * Loop over the major, minor and patch version numbers.
+   *
+   * Most of the conversion code is exactly the same, only error codes,
+   * messages and preludes (i.e., things to do before actually converting the
+   * version number) differ slightly.
+   *
+   * We can usually abstract this with a bit (a lot?) of preprocessor magic.
+   */
+  for (size_t num_i = 0; num_i < 3; ++num_i) {
+    const gchar *end_search = NULL;
+    if (0 == num_i) {
+      /* Scan for major version number separator (dot). */
+      end_search = g_strstr_len (tmp, -1, ".");
+      if (!(end_search)) {
+        /* No separator is an error. */
+        g_set_error_literal (gerr, ERROR_QUARK, X2GOCLIENT_OPENSSH_VERSION_ERROR_NO_MAJOR_SEPARATOR, "Version string does not contain major version number separator.");
+
+        vers_err = TRUE;
+
+        break;
+      }
+    }
+    else if (1 == num_i) {
+      if ('.' != *tmp) {
+        g_set_error_literal (gerr, ERROR_QUARK, X2GOCLIENT_OPENSSH_VERSION_ERROR_NOT_AT_MAJOR_SEPARATOR, "Major version number parsing did not stop at separator.");
 
-      x2goclient_openssh_version_free (struct_work_copy);
-      struct_work_copy = NULL;
+        vers_err = TRUE;
 
-      return;
-    }
-  }
+        break;
+      }
+      else {
+        /* Skip dot character. */
+        ++tmp;
+      }
 
-  /* Copy and convert major version number. */
-  gchar *tmp_copy = g_strndup (tmp, dot_search - tmp);
-  struct_work_copy->major = -1;
+      /* Scan for minor version number separator (p). */
+      end_search = g_strstr_len (tmp, -1, "p");
+      if (!(end_search)) {
+        /* No separator is an error. */
+        g_set_error_literal (gerr, ERROR_QUARK, X2GOCLIENT_OPENSSH_VERSION_ERROR_NO_MINOR_SEPARATOR, "Version string does not contain minor version number separator.");
 
-  _Bool conv_err = TRUE,
-        min_err = TRUE,
-        max_err = TRUE;
-  const gchar *end = NULL;
-  long long conv = x2goclient_str_to_int (tmp_copy, TRUE, 0, TRUE, G_MAXUINT32, &end, &conv_err, &min_err, &max_err);
+        vers_err = TRUE;
 
-  if (conv_err) {
-    if (min_err) {
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (MAJOR, UNDERFLOW), "Major version number is too small to fit into long long type.");
-    }
-    else if (max_err) {
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (MAJOR, OVERFLOW), "Major version number is too big to fit into long long type.");
+        break;
+      }
     }
     else {
-      end = NULL;
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (MAJOR, NOT_NUMERIC), "Major version number is not numeric.");
-    }
-  }
-  else {
-    if (min_err) {
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (MAJOR, NEGATIVE), "Major version number is not numeric.");
-    }
-    else if (max_err) {
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (MAJOR, TOO_BIG), "Major version number is too big.");
-    }
-    else {
-      struct_work_copy->major = conv;
-    }
-  }
-
-  if (end) {
-    /* Map end pointer to original string. */
-    tmp += (end - tmp_copy);
-  }
-
-  g_free (tmp_copy);
-  tmp_copy = NULL;
-
-  if (0 > struct_work_copy->major) {
-    x2goclient_openssh_version_free (struct_work_copy);
-    struct_work_copy = NULL;
-
-    return;
-  }
+      if ('p' != *tmp) {
+        g_set_error_literal (gerr, ERROR_QUARK, X2GOCLIENT_OPENSSH_VERSION_ERROR_NOT_AT_MINOR_SEPARATOR, "Minor version number parsing did not stop at separator.");
 
-  /* Copy and convert minor version number. */
-  tmp_copy = g_strdup (tmp);
-  struct_work_copy->minor = -1;
+        vers_err = TRUE;
 
-  conv_err = min_err = max_err = TRUE;
-  end = NULL;
-  conv = x2goclient_str_to_int (tmp_copy, TRUE, 0, TRUE, G_MAXUINT32, &end, &conv_err, &min_err, &max_err);
+        break;
+      }
+      else {
+        /* Skip 'p' character. */
+        ++tmp;
+      }
 
-  if (conv_err) {
-    if (min_err) {
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (MINOR, UNDERFLOW), "Minor version number is too small to fit into long long type.");
-    }
-    else if (max_err) {
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (MINOR, OVERFLOW), "Minor version number is too big to fit into long long type.");
-    }
-    else {
-      end = NULL;
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (MINOR, NOT_NUMERIC), "Minor version number is not numeric.");
-    }
-  }
-  else {
-    if (min_err) {
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (MINOR, NEGATIVE), "Minor version number is not numeric.");
-    }
-    else if (max_err) {
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (MINOR, TOO_BIG), "Minor version number is too big.");
-    }
-    else {
-      struct_work_copy->minor = conv;
+      /*
+       * Set end to actual end of string. We'll parse the patch version number
+       * opportunistically, since its terminator is not typically fixed, but
+       * might be one of multiple characters.
+       *
+       * In the worst case, i.e., if there are no characters left, end_search
+       * will point to tmp.
+       */
+      end_search = tmp + strlen (tmp);
     }
-  }
 
-  if (end) {
-    /* Map end pointer to original string. */
-    tmp += (end - tmp_copy);
-  }
+    g_assert (end_search);
 
-  g_free (tmp_copy);
-  tmp_copy = NULL;
+    if (2 > num_i) {
+      g_assert (tmp < end_search);
 
-  /*
-   * Handle error and additionally scan for the next character which MUST be a
-   * 'p' according to our scheme.
-   */
-  if ((0 > struct_work_copy->minor) || ('p' != *tmp)) {
-    x2goclient_openssh_version_free (struct_work_copy);
-    struct_work_copy = NULL;
+      /* Sanity check on version number - must be numeric/all digits. */
+      const gchar *int_search = NULL;
+      for (int_search = tmp; int_search < end_search; ++int_search) {
+        if (!(g_ascii_isdigit (*int_search))) {
+          gint err_code = -1;
+          const gchar *err_msg = NULL;
 
-    return;
-  }
-  else {
-    /* Skip 'p' character. */
-    ++tmp;
-  }
+          GENERATE_ERROR_CODE (err_code, num_i, NOT_NUMERIC);
+          GENERATE_ERROR_MSG (err_msg, num_i, "version number is not numeric.");
 
-  /* Copy and convert patch version number. */
-  tmp_copy = g_strdup (tmp);
-  struct_work_copy->patch = -1;
+          g_set_error_literal (gerr, ERROR_QUARK, err_code, err_msg);
 
-  conv_err = min_err = max_err = TRUE;
-  end = NULL;
-  conv = x2goclient_str_to_int (tmp_copy, TRUE, 0, TRUE, G_MAXUINT32, &end, &conv_err, &min_err, &max_err);
+          vers_err = TRUE;
 
-  if (conv_err) {
-    if (min_err) {
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (PATCH, UNDERFLOW), "Patch version number is too small to fit into long long type.");
+          break;
+        }
+      }
     }
-    else if (max_err) {
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (PATCH, OVERFLOW), "Patch version number is too big to fit into long long type.");
+
+    /* Copy and convert version number. */
+    gchar *tmp_copy = g_strndup (tmp, end_search - tmp);
+    SET_VER_COMPONENT (struct_work_copy, num_i, -1);
+
+    _Bool conv_err = TRUE,
+          min_err = TRUE,
+          max_err = TRUE;
+    const gchar *end = NULL;
+    const gchar *err_msg = NULL;
+    gint err_code = -1;
+    long long conv = x2goclient_str_to_int (tmp_copy, TRUE, 0, TRUE, G_MAXUINT32, &end, &conv_err, &min_err, &max_err);
+
+    if (conv_err) {
+      if (min_err) {
+        GENERATE_ERROR_CODE (err_code, num_i, UNDERFLOW);
+        GENERATE_ERROR_MSG (err_msg, num_i, "version number is too small to fit into long long type.");
+      }
+      else if (max_err) {
+        GENERATE_ERROR_CODE (err_code, num_i, OVERFLOW);
+        GENERATE_ERROR_MSG (err_msg, num_i, "version number is too big to fit into long long type.");
+      }
+      else {
+        GENERATE_ERROR_CODE (err_code, num_i, NOT_NUMERIC);
+        GENERATE_ERROR_MSG (err_msg, num_i, "version number is not numeric.");
+
+        end = NULL;
+      }
+
+      g_set_error_literal (gerr, ERROR_QUARK, err_code, err_msg);
     }
     else {
-      end = NULL;
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (PATCH, NOT_NUMERIC), "Patch version number is not numeric.");
-    }
-  }
-  else {
-    if (min_err) {
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (PATCH, NEGATIVE), "Patch version number is not numeric.");
-    }
-    else if (max_err) {
-      g_set_error_literal (gerr, ERROR_QUARK, GENERATE_ERROR (PATCH, TOO_BIG), "Patch version number is too big.");
+      if (min_err) {
+        GENERATE_ERROR_CODE (err_code, num_i, NEGATIVE);
+        GENERATE_ERROR_MSG (err_msg, num_i, "version number is not numeric.");
+
+        g_set_error_literal (gerr, ERROR_QUARK, err_code, err_msg);
+      }
+      else if (max_err) {
+        GENERATE_ERROR_CODE (err_code, num_i, TOO_BIG);
+        GENERATE_ERROR_MSG (err_msg, num_i, "version number is too big.");
+
+        g_set_error_literal (gerr, ERROR_QUARK, err_code, err_msg);
+      }
+      else {
+        SET_VER_COMPONENT (struct_work_copy, num_i, conv);
+      }
     }
-    else {
-      struct_work_copy->patch = conv;
+
+    if (end) {
+      /* Map end pointer to original string. */
+      tmp += (end - tmp_copy);
     }
-  }
 
-  if (end) {
-    /* Map end pointer to original string. */
-    tmp += (end - tmp_copy);
-  }
+    g_free (tmp_copy);
+    tmp_copy = NULL;
 
-  g_free (tmp_copy);
-  tmp_copy = NULL;
+    CHECK_VER_COMPONENT (struct_work_copy, num_i, vers_err);
+  }
 
-  if (0 > struct_work_copy->patch) {
+  if (vers_err) {
     x2goclient_openssh_version_free (struct_work_copy);
     struct_work_copy = NULL;
-
-    return;
   }
 
   /* Scan for a comma which should follow any additional version data. */
-  end = g_strstr_len (tmp, -1, ",");
+  const gchar *end = g_strstr_len (tmp, -1, ",");
   if (!(end)) {
     /*
      * Alternatively, scan for whitespace and we'll treat that as the
@@ -274,8 +320,8 @@ void x2goclient_openssh_version_parse (X2GoClientOpenSSHVersion *openssh_version
 
     if (!(end)) {
       /*
-       * No separator means or trailing whitespace means that we'll have to
-       * fast-forward to the actual end.
+       * No separator or trailing whitespace means that we'll have to fast-
+       * forward to the actual end.
        */
       end = (tmp + (strlen (tmp)));
     }
diff --git a/src/x2goclient-openssh-version.h b/src/x2goclient-openssh-version.h
index fcb5f83..0226ea2 100644
--- a/src/x2goclient-openssh-version.h
+++ b/src/x2goclient-openssh-version.h
@@ -55,6 +55,9 @@ enum {
   X2GOCLIENT_OPENSSH_VERSION_ERROR_INVALID_VERSION_STRING,
   X2GOCLIENT_OPENSSH_VERSION_ERROR_INVALID_PREAMBLE,
   X2GOCLIENT_OPENSSH_VERSION_ERROR_NO_MAJOR_SEPARATOR,
+  X2GOCLIENT_OPENSSH_VERSION_ERROR_NO_MINOR_SEPARATOR,
+  X2GOCLIENT_OPENSSH_VERSION_ERROR_NOT_AT_MAJOR_SEPARATOR,
+  X2GOCLIENT_OPENSSH_VERSION_ERROR_NOT_AT_MINOR_SEPARATOR,
   X2GOCLIENT_OPENSSH_VERSION_ERROR_MAJOR_NOT_NUMERIC,
   X2GOCLIENT_OPENSSH_VERSION_ERROR_MAJOR_UNDERFLOW,
   X2GOCLIENT_OPENSSH_VERSION_ERROR_MAJOR_OVERFLOW,

--
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