From f16b26bd6d31b3d91f1f441ebf85d9d58584c04a Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Tue, 16 Jun 2026 18:03:50 +0300 Subject: [PATCH] fix(review): address PR #130 review - proftest: declare the _mkdir_recursive loop index inside the for. - xmpp/presence: shorten the resource-snapshot comment to one line. - command/cmd_funcs: report the /vcard remove index error via cons_show_error. - common: replace g_assert in g_diff_to_gsize with a log_error + return 0 (moved out of the header into common.c); take get_random_string length as size_t instead of asserting non-negative. - otr: drop the change-narrating comment on the whitespace memmove. Refs #112 --- src/command/cmd_funcs.c | 2 +- src/common.c | 17 +++++++++++++---- src/common.h | 11 ++--------- src/otr/otr.c | 4 +--- src/xmpp/presence.c | 5 +---- tests/functionaltests/proftest.c | 3 +-- 6 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 827212f1..3fad000a 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -10066,7 +10066,7 @@ cmd_vcard_remove(ProfWin* window, const char* const command, gchar** args) int index; auto_gchar gchar* err_msg = NULL; if (!strtoi_range(args[1], &index, 0, INT_MAX, &err_msg)) { - cons_show("%s", err_msg); + cons_show_error("%s", err_msg); return TRUE; } vcard_user_remove_element((unsigned int)index); diff --git a/src/common.c b/src/common.c index bc70b2a3..477c41b4 100644 --- a/src/common.c +++ b/src/common.c @@ -309,6 +309,16 @@ str_replace(const char* string, const char* substr, return newstr; } +gsize +g_diff_to_gsize(const void* end, const void* start) +{ + if (end < start) { + log_error("g_diff_to_gsize: end < start (%p < %p)", end, start); + return 0; + } + return (gsize)((const char*)end - (const char*)start); +} + gboolean strtoi_range(const char* str, int* saveptr, int min, int max, gchar** err_msg) { @@ -690,19 +700,18 @@ get_file_paths_recursive(const char* path, GSList** contents) } gchar* -get_random_string(int length) +get_random_string(size_t length) { - g_assert(length >= 0); GRand* prng; gchar* rand; gchar alphabet[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"; int endrange = sizeof(alphabet) - 1; - rand = g_malloc0((gsize)length + 1); + rand = g_malloc0(length + 1); prng = g_rand_new(); - for (int i = 0; i < length; i++) { + for (size_t i = 0; i < length; i++) { rand[i] = alphabet[g_rand_int_range(prng, 0, endrange)]; } g_rand_free(prng); diff --git a/src/common.h b/src/common.h index efe89cf9..7abcb9fd 100644 --- a/src/common.h +++ b/src/common.h @@ -158,14 +158,7 @@ gboolean create_dir(const char* name); gboolean copy_file(const char* const src, const char* const target, const gboolean overwrite_existing); char* str_replace(const char* string, const char* substr, const char* replacement); gboolean strtoi_range(const char* str, int* saveptr, int min, int max, char** err_msg); - -// Pointer-diff to gsize for callers that know end >= start by construction. -static inline gsize -g_diff_to_gsize(const void* end, const void* start) -{ - g_assert(end >= start); - return (gsize)((const char*)end - (const char*)start); -} +gsize g_diff_to_gsize(const void* end, const void* start); int utf8_display_len(const char* const str); gchar* str_xml_sanitize(const char* const str); @@ -186,7 +179,7 @@ int is_regular_file(const char* path); int is_dir(const char* path); void get_file_paths_recursive(const char* directory, GSList** contents); -gchar* get_random_string(int length); +gchar* get_random_string(size_t length); gboolean call_external(gchar** argv); gchar** format_call_external_argv(const char* template_fmt, const char* url, const char* filename); diff --git a/src/otr/otr.c b/src/otr/otr.c index ad382148..b5c336a9 100644 --- a/src/otr/otr.c +++ b/src/otr/otr.c @@ -278,9 +278,7 @@ otr_on_message_recv(const char* const barejid, const char* const resource, const if (strstr(message, OTRL_MESSAGE_TAG_V2) && strstr(message, OTRL_MESSAGE_TAG_V1)) { tag_length = 32; } - // Shift the remainder of the message (including the trailing NUL) - // left over the tag, not just the next tag_length bytes — otherwise - // the tail is left in place and the body is duplicated/truncated. + // Move the rest of the message (with NUL) over the tag. memmove(whitespace_base, whitespace_base + tag_length, strlen(whitespace_base + tag_length) + 1); char* otr_query_message = otr_start_query(); cons_show("OTR Whitespace pattern detected. Attempting to start OTR session…"); diff --git a/src/xmpp/presence.c b/src/xmpp/presence.c index c1a47765..8ab4625d 100644 --- a/src/xmpp/presence.c +++ b/src/xmpp/presence.c @@ -621,10 +621,7 @@ _available_handler(xmpp_stanza_t* const stanza) } if (g_strcmp0(xmpp_presence->jid->barejid, my_jid->barejid) == 0) { - // Snapshot everything we read from `resource` before ownership is - // transferred to the available-resources table; dereferencing it - // afterwards would be a use-after-free should the same resource name - // ever be re-added between the transfer and the reads below. + // Copy what we read before connection_add_available_resource() takes ownership. Resource* resource_for_roster = resource_copy(resource); auto_gchar gchar* resource_name = g_strdup(resource->name); auto_gchar gchar* resource_status = resource->status ? g_strdup(resource->status) : NULL; diff --git a/tests/functionaltests/proftest.c b/tests/functionaltests/proftest.c index d8cbe739..4b202c26 100644 --- a/tests/functionaltests/proftest.c +++ b/tests/functionaltests/proftest.c @@ -119,10 +119,9 @@ _create_dir(const char* name) gboolean _mkdir_recursive(const char* dir) { - size_t i; gboolean result = TRUE; - for (i = 1; i <= strlen(dir); i++) { + for (size_t i = 1; i <= strlen(dir); i++) { if (dir[i] == '/' || dir[i] == '\0') { gchar* next_dir = g_strndup(dir, i); result = _create_dir(next_dir);