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