From bc0e16945165824c1454d8fa835f4a70283ee854 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 31 Mar 2025 15:36:51 +0100 Subject: [PATCH] Rationalise g_htoi() / xrdp_wm_htoi() xrdp contains two functions which do similar things:- - g_htoi() converts a hex string to an integer, ignoring unrecognised characters - xrdp_wm_htoi() converts a hex string to an integer, ignoring leading whitespace, but terminating on unrecognised characters An analysis of the uses of g_htoi() shows that the only place where unrecognised characters might be encountered is parsing lines from xrdp_keyboard.ini, where all values have an '0x' prefix (i.e. the 'x' is unrecognised) An analysis of xrdp_wm_htoi() shows that the functionality to ignore leading whitespace is not used. Both functions are replaced with a re-written g_htoi() which is const- correct and provided with test cases. This function behaves in the same way as the atoi() library function, in that it terminates on an unexpected character. The use of g_htoi() in parsing lines from xrdp_keyboard.ini is replaced with a call to g_atoix() which handles the '0x' prefix correctly. --- common/string_calls.c | 94 ++++++++------------------------ common/string_calls.h | 10 +++- tests/common/test_string_calls.c | 27 +++++++++ xrdp/lang.c | 4 +- xrdp/xrdp.h | 2 - xrdp/xrdp_login_wnd.c | 26 ++++----- xrdp/xrdp_wm.c | 54 +++--------------- 7 files changed, 82 insertions(+), 135 deletions(-) diff --git a/common/string_calls.c b/common/string_calls.c index 9079be5a..99ec4b40 100644 --- a/common/string_calls.c +++ b/common/string_calls.c @@ -502,82 +502,32 @@ size_t strlcpy(char *dst, const char *src, size_t dsize) #endif /*****************************************************************************/ -int -g_htoi(char *str) +unsigned int +g_htoi(const char *str) { - int len; - int index; - int rv; - int val; - int shift; - - rv = 0; - len = strlen(str); - index = len - 1; - shift = 0; - - while (index >= 0) + unsigned int rv = 0; + while (*str != '\0') { - val = 0; - - switch (str[index]) + char c = *str; + unsigned int val; + if (c >= '0' && c <= '9') { - case '1': - val = 1; - break; - case '2': - val = 2; - break; - case '3': - val = 3; - break; - case '4': - val = 4; - break; - case '5': - val = 5; - break; - case '6': - val = 6; - break; - case '7': - val = 7; - break; - case '8': - val = 8; - break; - case '9': - val = 9; - break; - case 'a': - case 'A': - val = 10; - break; - case 'b': - case 'B': - val = 11; - break; - case 'c': - case 'C': - val = 12; - break; - case 'd': - case 'D': - val = 13; - break; - case 'e': - case 'E': - val = 14; - break; - case 'f': - case 'F': - val = 15; - break; + val = c - '0'; } - - rv = rv | (val << shift); - index--; - shift += 4; + else if (c >= 'A' && c <= 'F') + { + val = (c - 'A' + 10); + } + else if (c >= 'a' && c <= 'f') + { + val = (c - 'a' + 10); + } + else + { + break; // Unrecognised character + } + rv = (rv << 4) | val; + ++str; } return rv; diff --git a/common/string_calls.h b/common/string_calls.h index f8644bb1..77e26f3d 100644 --- a/common/string_calls.h +++ b/common/string_calls.h @@ -310,7 +310,15 @@ size_t strlcpy(char *dst, const char *src, size_t dsize); * @return int Integer expression of a string */ int g_atoix(const char *str); -int g_htoi(char *str); +/** + * Converts a hex string to an integer + * @param str pointer to Hex string containing only hex digits + * @return value, converted from hex + * + * This function is intended to be used as an analogue to the standard + * function atoi(). It performs no error checking of its own + */ +unsigned int g_htoi(const char *str); int g_bytes_to_hexstr(const void *bytes, int num_bytes, char *out_str, int bytes_out_str); int g_pos(const char *str, const char *to_find); diff --git a/tests/common/test_string_calls.c b/tests/common/test_string_calls.c index 5944c536..841a456d 100644 --- a/tests/common/test_string_calls.c +++ b/tests/common/test_string_calls.c @@ -1141,6 +1141,28 @@ END_TEST /******************************************************************************/ +START_TEST(test_htoi__all) +{ + // Invalid 1st character + ck_assert_int_eq(g_htoi(""), 0); + ck_assert_int_eq(g_htoi("z"), 0); + // Test border conditions (assumes ASCII) */ + ck_assert_int_eq(g_htoi("99/"), 0x99); /* '/' = one-before '0' */ + ck_assert_int_eq(g_htoi("99:"), 0x99); /* '/' = one-after '9' */ + ck_assert_int_eq(g_htoi("99@"), 0x99); /* '#' = one-before 'A' */ + ck_assert_int_eq(g_htoi("99G"), 0x99); /* 'G' = one-after 'F' */ + ck_assert_int_eq(g_htoi("99`"), 0x99); /* '`' = one-before 'a' */ + ck_assert_int_eq(g_htoi("99g"), 0x99); /* 'g' = one-after 'f' */ + + // Test all valid characters and shifting is OK */ + ck_assert_int_eq(g_htoi("01234567"), 0x01234567); + ck_assert_int_eq(g_htoi("8ABCDEFa"), 0x8ABCDEFa); + ck_assert_int_eq(g_htoi("bcdef"), 0xbcdef); +} +END_TEST + +/******************************************************************************/ + Suite * make_suite_test_string(void) { @@ -1152,6 +1174,7 @@ make_suite_test_string(void) TCase *tc_char2bm; TCase *tc_strtrim; TCase *tc_sigs; + TCase *tc_htoi; s = suite_create("String"); @@ -1230,5 +1253,9 @@ make_suite_test_string(void) tcase_add_test(tc_sigs, test_sigs__common); tcase_add_test(tc_sigs, test_sigs__bigint); + tc_htoi = tcase_create("g_htoi"); + suite_add_tcase(s, tc_htoi); + tcase_add_test(tc_htoi, test_htoi__all); + return s; } diff --git a/xrdp/lang.c b/xrdp/lang.c index aa606863..a94ef99c 100644 --- a/xrdp/lang.c +++ b/xrdp/lang.c @@ -698,7 +698,7 @@ xrdp_init_xkb_layout(struct xrdp_client_info *client_info) int rdp_layout_id; item = (char *)list_get_item(items, index); value = (char *)list_get_item(values, index); - rdp_layout_id = g_htoi(value); + rdp_layout_id = g_atoix(value); if (rdp_layout_id == client_info->keylayout) { g_strncpy(rdp_layout, item, 255); @@ -714,7 +714,7 @@ xrdp_init_xkb_layout(struct xrdp_client_info *client_info) { item = (char *)list_get_item(items, index); value = (char *)list_get_item(values, index); - int rdp_layout_id = g_htoi(value); + int rdp_layout_id = g_atoix(value); if (rdp_layout_id == alt_layout) { g_strncpy(rdp_layout, item, 255); diff --git a/xrdp/xrdp.h b/xrdp/xrdp.h index d4f1c8c4..ccff3930 100644 --- a/xrdp/xrdp.h +++ b/xrdp/xrdp.h @@ -141,8 +141,6 @@ xrdp_wm_send_bitmap(struct xrdp_wm *self, struct xrdp_bitmap *bitmap, int x, int y, int cx, int cy); int xrdp_wm_set_pointer(struct xrdp_wm *self, int cache_idx); -unsigned int -xrdp_wm_htoi (const char *ptr); int xrdp_wm_set_focused(struct xrdp_wm *self, struct xrdp_bitmap *wnd); int diff --git a/xrdp/xrdp_login_wnd.c b/xrdp/xrdp_login_wnd.c index 650be6bc..80d0a737 100644 --- a/xrdp/xrdp_login_wnd.c +++ b/xrdp/xrdp_login_wnd.c @@ -1074,8 +1074,8 @@ load_xrdp_config(struct xrdp_config *config, const char *xrdp_ini, int bpp) globals->ini_version = 1; globals->default_dpi = 96; - globals->ls_top_window_bg_color = HCOLOR(bpp, xrdp_wm_htoi("009cb5")); - globals->ls_bg_color = HCOLOR(bpp, xrdp_wm_htoi("dedede")); + globals->ls_top_window_bg_color = HCOLOR(bpp, g_htoi("009cb5")); + globals->ls_bg_color = HCOLOR(bpp, g_htoi("dedede")); globals->ls_unscaled.width = 350; globals->ls_unscaled.height = 350; globals->ls_background_transform = XBLT_NONE; @@ -1214,47 +1214,47 @@ load_xrdp_config(struct xrdp_config *config, const char *xrdp_ini, int bpp) else if (g_strncmp(n, "grey", 64) == 0) { - globals->grey = xrdp_wm_htoi(v); + globals->grey = g_htoi(v); } else if (g_strncmp(n, "black", 64) == 0) { - globals->black = xrdp_wm_htoi(v); + globals->black = g_htoi(v); } else if (g_strncmp(n, "dark_grey", 64) == 0) { - globals->dark_grey = xrdp_wm_htoi(v); + globals->dark_grey = g_htoi(v); } else if (g_strncmp(n, "blue", 64) == 0) { - globals->blue = xrdp_wm_htoi(v); + globals->blue = g_htoi(v); } else if (g_strncmp(n, "dark_blue", 64) == 0) { - globals->dark_blue = xrdp_wm_htoi(v); + globals->dark_blue = g_htoi(v); } else if (g_strncmp(n, "white", 64) == 0) { - globals->white = xrdp_wm_htoi(v); + globals->white = g_htoi(v); } else if (g_strncmp(n, "red", 64) == 0) { - globals->red = xrdp_wm_htoi(v); + globals->red = g_htoi(v); } else if (g_strncmp(n, "green", 64) == 0) { - globals->green = xrdp_wm_htoi(v); + globals->green = g_htoi(v); } else if (g_strncmp(n, "background", 64) == 0) { - globals->background = xrdp_wm_htoi(v); + globals->background = g_htoi(v); } /* misc stuff */ @@ -1313,7 +1313,7 @@ load_xrdp_config(struct xrdp_config *config, const char *xrdp_ini, int bpp) else if (g_strncmp(n, "ls_top_window_bg_color", 64) == 0) { - globals->ls_top_window_bg_color = HCOLOR(bpp, xrdp_wm_htoi(v)); + globals->ls_top_window_bg_color = HCOLOR(bpp, g_htoi(v)); } else if (g_strncmp(n, "ls_width", 64) == 0) @@ -1328,7 +1328,7 @@ load_xrdp_config(struct xrdp_config *config, const char *xrdp_ini, int bpp) else if (g_strncmp(n, "ls_bg_color", 64) == 0) { - globals->ls_bg_color = HCOLOR(bpp, xrdp_wm_htoi(v)); + globals->ls_bg_color = HCOLOR(bpp, g_htoi(v)); } else if (g_strncmp(n, "ls_title", 255) == 0) diff --git a/xrdp/xrdp_wm.c b/xrdp/xrdp_wm.c index e9a3bd6d..9ad8846a 100644 --- a/xrdp/xrdp_wm.c +++ b/xrdp/xrdp_wm.c @@ -442,42 +442,6 @@ xrdp_wm_set_pointer(struct xrdp_wm *self, int cache_idx) return libxrdp_set_pointer(self->session, cache_idx); } -/*****************************************************************************/ -/* convert hex string to int */ -unsigned int -xrdp_wm_htoi (const char *ptr) -{ - unsigned int value = 0; - char ch = *ptr; - - while (ch == ' ' || ch == '\t') - { - ch = *(++ptr); - } - - for (;;) - { - if (ch >= '0' && ch <= '9') - { - value = (value << 4) + (ch - '0'); - } - else if (ch >= 'A' && ch <= 'F') - { - value = (value << 4) + (ch - 'A' + 10); - } - else if (ch >= 'a' && ch <= 'f') - { - value = (value << 4) + (ch - 'a' + 10); - } - else - { - return value; - } - - ch = *(++ptr); - } -} - /*****************************************************************************/ int xrdp_wm_load_static_colors_plus(struct xrdp_wm *self, char *autorun_name) @@ -529,47 +493,47 @@ xrdp_wm_load_static_colors_plus(struct xrdp_wm *self, char *autorun_name) if (g_strcasecmp(val, "black") == 0) { val = (char *)list_get_item(values, index); - self->black = HCOLOR(self->screen->bpp, xrdp_wm_htoi(val)); + self->black = HCOLOR(self->screen->bpp, g_htoi(val)); } else if (g_strcasecmp(val, "grey") == 0) { val = (char *)list_get_item(values, index); - self->grey = HCOLOR(self->screen->bpp, xrdp_wm_htoi(val)); + self->grey = HCOLOR(self->screen->bpp, g_htoi(val)); } else if (g_strcasecmp(val, "dark_grey") == 0) { val = (char *)list_get_item(values, index); - self->dark_grey = HCOLOR(self->screen->bpp, xrdp_wm_htoi(val)); + self->dark_grey = HCOLOR(self->screen->bpp, g_htoi(val)); } else if (g_strcasecmp(val, "blue") == 0) { val = (char *)list_get_item(values, index); - self->blue = HCOLOR(self->screen->bpp, xrdp_wm_htoi(val)); + self->blue = HCOLOR(self->screen->bpp, g_htoi(val)); } else if (g_strcasecmp(val, "dark_blue") == 0) { val = (char *)list_get_item(values, index); - self->dark_blue = HCOLOR(self->screen->bpp, xrdp_wm_htoi(val)); + self->dark_blue = HCOLOR(self->screen->bpp, g_htoi(val)); } else if (g_strcasecmp(val, "white") == 0) { val = (char *)list_get_item(values, index); - self->white = HCOLOR(self->screen->bpp, xrdp_wm_htoi(val)); + self->white = HCOLOR(self->screen->bpp, g_htoi(val)); } else if (g_strcasecmp(val, "red") == 0) { val = (char *)list_get_item(values, index); - self->red = HCOLOR(self->screen->bpp, xrdp_wm_htoi(val)); + self->red = HCOLOR(self->screen->bpp, g_htoi(val)); } else if (g_strcasecmp(val, "green") == 0) { val = (char *)list_get_item(values, index); - self->green = HCOLOR(self->screen->bpp, xrdp_wm_htoi(val)); + self->green = HCOLOR(self->screen->bpp, g_htoi(val)); } else if (g_strcasecmp(val, "background") == 0) { val = (char *)list_get_item(values, index); - self->background = HCOLOR(self->screen->bpp, xrdp_wm_htoi(val)); + self->background = HCOLOR(self->screen->bpp, g_htoi(val)); } else if (g_strcasecmp(val, "autorun") == 0) {