From b36562c6b67aea7d691e6c8e2373b3f94a9bf623 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Thu, 27 Jul 2023 12:24:32 +0200 Subject: [PATCH 1/2] Introduce `win_println_va()` Previously we printed a format string into a `GString` to then print that result again into a `GString` and display that. This patch removes one level of `GString` printing by forwarding the `va_arg` to the newly added API `win_println_va()`. Introducing `static win_println_va_internal()` also allowed refactoring most of the `win_print*()` and `win_append*()` implementations and removing all the redundant code. Signed-off-by: Steffen Jaeckel --- src/ui/console.c | 15 +---- src/ui/ui.h | 2 + src/ui/window.c | 140 +++++++++++++---------------------------------- 3 files changed, 43 insertions(+), 114 deletions(-) diff --git a/src/ui/console.c b/src/ui/console.c index efdc77b7..c696d28b 100644 --- a/src/ui/console.c +++ b/src/ui/console.c @@ -73,14 +73,10 @@ static GList* alert_list; void cons_debug(const char* const msg, ...) { - ProfWin* console = wins_get_console(); if (strcmp(PACKAGE_STATUS, "development") == 0) { va_list arg; va_start(arg, msg); - GString* fmt_msg = g_string_new(NULL); - g_string_vprintf(fmt_msg, msg, arg); - win_println(console, THEME_DEFAULT, "-", "%s", fmt_msg->str); - g_string_free(fmt_msg, TRUE); + win_println_va(wins_get_console(), THEME_DEFAULT, "-", msg, arg); va_end(arg); } } @@ -88,25 +84,20 @@ cons_debug(const char* const msg, ...) void cons_show(const char* const msg, ...) { - ProfWin* console = wins_get_console(); va_list arg; va_start(arg, msg); - GString* fmt_msg = g_string_new(NULL); - g_string_vprintf(fmt_msg, msg, arg); - win_println(console, THEME_DEFAULT, "-", "%s", fmt_msg->str); - g_string_free(fmt_msg, TRUE); + win_println_va(wins_get_console(), THEME_DEFAULT, "-", msg, arg); va_end(arg); } void cons_show_padded(int pad, const char* const msg, ...) { - ProfWin* console = wins_get_console(); va_list arg; va_start(arg, msg); GString* fmt_msg = g_string_new(NULL); g_string_vprintf(fmt_msg, msg, arg); - win_println_indent(console, pad, "%s", fmt_msg->str); + win_println_indent(wins_get_console(), pad, "%s", fmt_msg->str); g_string_free(fmt_msg, TRUE); va_end(arg); } diff --git a/src/ui/ui.h b/src/ui/ui.h index 2c4d4c99..cb5272ed 100644 --- a/src/ui/ui.h +++ b/src/ui/ui.h @@ -393,6 +393,8 @@ void win_print(ProfWin* window, theme_item_t theme_item, const char* show_char, void win_println(ProfWin* window, theme_item_t theme_item, const char* show_char, const char* const message, ...); void win_println_indent(ProfWin* window, int pad, const char* const message, ...); +void win_println_va(ProfWin* window, theme_item_t theme_item, const char* show_char, const char* const message, va_list arg); + void win_append(ProfWin* window, theme_item_t theme_item, const char* const message, ...); void win_appendln(ProfWin* window, theme_item_t theme_item, const char* const message, ...); diff --git a/src/ui/window.c b/src/ui/window.c index 0546c130..971711ea 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -340,17 +340,17 @@ win_get_title(ProfWin* window) gboolean show_titlebar_jid = prefs_get_boolean(PREF_TITLEBAR_MUC_TITLE_JID); gboolean show_titlebar_name = prefs_get_boolean(PREF_TITLEBAR_MUC_TITLE_NAME); - GString* title = g_string_new(""); if (show_titlebar_name && mucwin->room_name) { - g_string_append(title, mucwin->room_name); - g_string_append(title, " "); - } - if (show_titlebar_jid) { - g_string_append(title, mucwin->roomjid); + if (show_titlebar_jid) + return g_strdup_printf("%s %s", mucwin->room_name, mucwin->roomjid); + else + return g_strdup(mucwin->room_name); } + if (show_titlebar_jid) + return g_strdup(mucwin->roomjid); - return g_string_free(title, FALSE); + return g_strdup(""); } case WIN_CONFIG: { @@ -1536,143 +1536,86 @@ win_print_old_history(ProfWin* window, const ProfMessage* const message) g_date_time_unref(message->timestamp); } -void -win_print(ProfWin* window, theme_item_t theme_item, const char* show_char, const char* const message, ...) +static void +win_println_va_internal(ProfWin* window, theme_item_t theme_item, int pad, int flags, const char* show_char, const char* const message, va_list arg) { GDateTime* timestamp = g_date_time_new_now_local(); - va_list arg; - va_start(arg, message); - GString* fmt_msg = g_string_new(NULL); - g_string_vprintf(fmt_msg, message, arg); + auto_gchar gchar* msg = g_strdup_vprintf(message, arg); - buffer_append(window->layout->buffer, show_char, 0, timestamp, NO_EOL, theme_item, "", NULL, fmt_msg->str, NULL, NULL); - _win_print_internal(window, show_char, 0, timestamp, NO_EOL, theme_item, "", fmt_msg->str, NULL); + buffer_append(window->layout->buffer, show_char, pad, timestamp, flags, theme_item, "", NULL, msg, NULL, NULL); + _win_print_internal(window, show_char, pad, timestamp, flags, theme_item, "", msg, NULL); inp_nonblocking(TRUE); g_date_time_unref(timestamp); +} - g_string_free(fmt_msg, TRUE); +void +win_print(ProfWin* window, theme_item_t theme_item, const char* show_char, const char* const message, ...) +{ + va_list arg; + va_start(arg, message); + win_println_va_internal(window, theme_item, 0, NO_EOL, show_char, message, arg); va_end(arg); } +void +win_println_va(ProfWin* window, theme_item_t theme_item, const char* show_char, const char* const message, va_list arg) +{ + win_println_va_internal(window, theme_item, 0, 0, show_char, message, arg); +} + void win_println(ProfWin* window, theme_item_t theme_item, const char* show_char, const char* const message, ...) { - GDateTime* timestamp = g_date_time_new_now_local(); - va_list arg; va_start(arg, message); - GString* fmt_msg = g_string_new(NULL); - g_string_vprintf(fmt_msg, message, arg); - - buffer_append(window->layout->buffer, show_char, 0, timestamp, 0, theme_item, "", NULL, fmt_msg->str, NULL, NULL); - _win_print_internal(window, show_char, 0, timestamp, 0, theme_item, "", fmt_msg->str, NULL); - - inp_nonblocking(TRUE); - g_date_time_unref(timestamp); - - g_string_free(fmt_msg, TRUE); + win_println_va_internal(window, theme_item, 0, 0, show_char, message, arg); va_end(arg); } void win_println_indent(ProfWin* window, int pad, const char* const message, ...) { - GDateTime* timestamp = g_date_time_new_now_local(); - va_list arg; va_start(arg, message); - GString* fmt_msg = g_string_new(NULL); - g_string_vprintf(fmt_msg, message, arg); - - buffer_append(window->layout->buffer, "-", pad, timestamp, 0, THEME_DEFAULT, "", NULL, fmt_msg->str, NULL, NULL); - _win_print_internal(window, "-", pad, timestamp, 0, THEME_DEFAULT, "", fmt_msg->str, NULL); - - inp_nonblocking(TRUE); - g_date_time_unref(timestamp); - - g_string_free(fmt_msg, TRUE); + win_println_va_internal(window, THEME_DEFAULT, pad, 0, "-", message, arg); va_end(arg); } void win_append(ProfWin* window, theme_item_t theme_item, const char* const message, ...) { - GDateTime* timestamp = g_date_time_new_now_local(); - va_list arg; va_start(arg, message); - GString* fmt_msg = g_string_new(NULL); - g_string_vprintf(fmt_msg, message, arg); - - buffer_append(window->layout->buffer, "-", 0, timestamp, NO_DATE | NO_EOL, theme_item, "", NULL, fmt_msg->str, NULL, NULL); - _win_print_internal(window, "-", 0, timestamp, NO_DATE | NO_EOL, theme_item, "", fmt_msg->str, NULL); - - inp_nonblocking(TRUE); - g_date_time_unref(timestamp); - - g_string_free(fmt_msg, TRUE); + win_println_va_internal(window, theme_item, 0, NO_DATE | NO_EOL, "-", message, arg); va_end(arg); } void win_appendln(ProfWin* window, theme_item_t theme_item, const char* const message, ...) { - GDateTime* timestamp = g_date_time_new_now_local(); - va_list arg; va_start(arg, message); - GString* fmt_msg = g_string_new(NULL); - g_string_vprintf(fmt_msg, message, arg); - - buffer_append(window->layout->buffer, "-", 0, timestamp, NO_DATE, theme_item, "", NULL, fmt_msg->str, NULL, NULL); - _win_print_internal(window, "-", 0, timestamp, NO_DATE, theme_item, "", fmt_msg->str, NULL); - - inp_nonblocking(TRUE); - g_date_time_unref(timestamp); - - g_string_free(fmt_msg, TRUE); + win_println_va_internal(window, theme_item, 0, NO_DATE, "-", message, arg); va_end(arg); } void win_append_highlight(ProfWin* window, theme_item_t theme_item, const char* const message, ...) { - GDateTime* timestamp = g_date_time_new_now_local(); - va_list arg; va_start(arg, message); - GString* fmt_msg = g_string_new(NULL); - g_string_vprintf(fmt_msg, message, arg); - - buffer_append(window->layout->buffer, "-", 0, timestamp, NO_DATE | NO_ME | NO_EOL, theme_item, "", NULL, fmt_msg->str, NULL, NULL); - _win_print_internal(window, "-", 0, timestamp, NO_DATE | NO_ME | NO_EOL, theme_item, "", fmt_msg->str, NULL); - - inp_nonblocking(TRUE); - g_date_time_unref(timestamp); - - g_string_free(fmt_msg, TRUE); + win_println_va_internal(window, theme_item, 0, NO_DATE | NO_ME | NO_EOL, "-", message, arg); va_end(arg); } void win_appendln_highlight(ProfWin* window, theme_item_t theme_item, const char* const message, ...) { - GDateTime* timestamp = g_date_time_new_now_local(); - va_list arg; va_start(arg, message); - GString* fmt_msg = g_string_new(NULL); - g_string_vprintf(fmt_msg, message, arg); - - buffer_append(window->layout->buffer, "-", 0, timestamp, NO_DATE | NO_ME, theme_item, "", NULL, fmt_msg->str, NULL, NULL); - _win_print_internal(window, "-", 0, timestamp, NO_DATE | NO_ME, theme_item, "", fmt_msg->str, NULL); - - inp_nonblocking(TRUE); - g_date_time_unref(timestamp); - - g_string_free(fmt_msg, TRUE); + win_println_va_internal(window, theme_item, 0, NO_DATE | NO_ME, "-", message, arg); va_end(arg); } @@ -1752,16 +1695,15 @@ _win_printf(ProfWin* window, const char* show_char, int pad_indent, GDateTime* t va_list arg; va_start(arg, message); - GString* fmt_msg = g_string_new(NULL); - g_string_vprintf(fmt_msg, message, arg); - buffer_append(window->layout->buffer, show_char, pad_indent, timestamp, flags, theme_item, display_from, from_jid, fmt_msg->str, NULL, message_id); - _win_print_internal(window, show_char, pad_indent, timestamp, flags, theme_item, display_from, fmt_msg->str, NULL); + auto_gchar gchar* msg = g_strdup_vprintf(message, arg); + + buffer_append(window->layout->buffer, show_char, pad_indent, timestamp, flags, theme_item, display_from, from_jid, msg, NULL, message_id); + _win_print_internal(window, show_char, pad_indent, timestamp, flags, theme_item, display_from, msg, NULL); inp_nonblocking(TRUE); g_date_time_unref(timestamp); - g_string_free(fmt_msg, TRUE); va_end(arg); } @@ -2221,12 +2163,10 @@ win_command_exec_error(ProfWin* window, const char* const command, const char* c assert(window != NULL); va_list arg; va_start(arg, error); - GString* msg = g_string_new(NULL); - g_string_vprintf(msg, error, arg); + auto_gchar gchar* msg = g_strdup_vprintf(error, arg); - win_println(window, THEME_ERROR, "!", "Error executing command %s: %s", command, msg->str); + win_println(window, THEME_ERROR, "!", "Error executing command %s: %s", command, msg); - g_string_free(msg, TRUE); va_end(arg); } @@ -2306,9 +2246,5 @@ win_quote_autocomplete(ProfWin* window, const char* const input, gboolean previo auto_gcharv gchar** parts = g_strsplit(result, "\n", -1); auto_gchar gchar* quoted_result = g_strjoinv("\n> ", parts); - GString* replace_with = g_string_new("> "); - g_string_append(replace_with, quoted_result); - g_string_append(replace_with, "\n"); - - return g_string_free(replace_with, FALSE); + return g_strdup_printf("> %s\n", quoted_result); } From d3b6ebb1d7cfd4f970977c72dc54455e7ef68a91 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Tue, 29 Aug 2023 10:37:10 +0200 Subject: [PATCH 2/2] Less allocations in `str_replace()` If `n` is the number of `substr` replace operations, the old implementation did `n+1` allocations and always first copied start, then replacement, then the remaining (unsearched) part of the original string. Now calculate the number of occurences of `substr` before allocating the resulting string. Change the algorithm to parse through the original string and only copy the parts that must be kept into the resulting string plus the replacements where they belong at. Now we also only have to go through the original string twice plus we only do a single allocation. Signed-off-by: Steffen Jaeckel --- src/common.c | 53 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/src/common.c b/src/common.c index 3924a260..5d6a1125 100644 --- a/src/common.c +++ b/src/common.c @@ -174,9 +174,12 @@ char* str_replace(const char* string, const char* substr, const char* replacement) { - char* tok = NULL; + const char* head = NULL; + const char* tok = NULL; char* newstr = NULL; - char* head = NULL; + char* wr = NULL; + size_t num_substr = 0; + size_t len_string, len_substr, len_replacement, len_string_remains, len_newstr; if (string == NULL) return NULL; @@ -184,26 +187,40 @@ str_replace(const char* string, const char* substr, if (substr == NULL || replacement == NULL || (strcmp(substr, "") == 0)) return strdup(string); - newstr = strdup(string); - head = newstr; + tok = string; + while ((tok = strstr(tok, substr))) { + num_substr++; + tok++; + } + + if (num_substr == 0) + return strdup(string); + + len_string = strlen(string); + len_substr = strlen(substr); + len_replacement = strlen(replacement); + len_newstr = len_string - (num_substr * len_substr) + (num_substr * len_replacement); + newstr = malloc(len_newstr + 1); + if (newstr == NULL) { + return NULL; + } + len_string_remains = len_string; + + head = string; + wr = newstr; while ((tok = strstr(head, substr))) { - auto_char char* oldstr = newstr; - newstr = malloc(strlen(oldstr) - strlen(substr) + strlen(replacement) + 1); + size_t l = tok - head; + memcpy(wr, head, l); + wr += l; + memcpy(wr, replacement, len_replacement); + wr += len_replacement; + len_string_remains -= len_substr + l; - if (newstr == NULL) { - return NULL; - } - - memcpy(newstr, oldstr, tok - oldstr); - memcpy(newstr + (tok - oldstr), replacement, strlen(replacement)); - memcpy(newstr + (tok - oldstr) + strlen(replacement), - tok + strlen(substr), - strlen(oldstr) - strlen(substr) - (tok - oldstr)); - memset(newstr + strlen(oldstr) - strlen(substr) + strlen(replacement), 0, 1); - - head = newstr + (tok - oldstr) + strlen(replacement); + head = tok + len_substr; } + memcpy(wr, head, len_string_remains); + newstr[len_newstr] = '\0'; return newstr; }