From 662a0be6334181a2d816f4be6de4affdf1e107b4 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Thu, 6 Mar 2025 21:16:06 +0100 Subject: [PATCH 01/13] Add `--cmd` option This allows to kind of automate what profanity should do as first jobs, e.g. `--cmd /foo --cmd /bar --cmd /quit` so one can easily check for memory leaks. Signed-off-by: Steffen Jaeckel --- docs/profanity.1 | 7 +++++++ src/main.c | 27 +++++++++++--------------- src/profanity.c | 49 +++++++++++++++++++++++++++++++++++++++++++----- src/profanity.h | 2 +- 4 files changed, 63 insertions(+), 22 deletions(-) diff --git a/docs/profanity.1 b/docs/profanity.1 index ef1f45412..a8ac092ef 100755 --- a/docs/profanity.1 +++ b/docs/profanity.1 @@ -36,6 +36,13 @@ may be set to DEBUG, INFO (the default), WARN or ERROR. .BI "\-f, \-\-logfile" Specify a different logfile .TP +.BI "\-r, \-\-cmd" +Specify the commands that should be run right after starting up. +This can be given multiple times, e.g. +.EX +profanity --cmd /foo --cmd "/sleep 10" --cmd /quit +.EE +.TP .BI "\-t, \-\-theme "THEME Specify which theme to use. .I THEME diff --git a/src/main.c b/src/main.c index 4edd5cb87..64f7e8e00 100644 --- a/src/main.c +++ b/src/main.c @@ -55,16 +55,17 @@ #include "common.h" #include "command/cmd_defs.h" -static gboolean version = FALSE; -static char* log = NULL; -static char* log_file = NULL; -static char* account_name = NULL; -static char* config_file = NULL; -static char* theme_name = NULL; - int main(int argc, char** argv) { + gboolean version = FALSE; + auto_gchar gchar* log = NULL; + auto_gchar gchar* log_file = NULL; + auto_gchar gchar* account_name = NULL; + auto_gchar gchar* config_file = NULL; + auto_gchar gchar* theme_name = NULL; + auto_gcharv gchar** commands = NULL; + if (argc == 2 && g_strcmp0(PACKAGE_STATUS, "development") == 0) { if (g_strcmp0(argv[1], "docgen") == 0) { command_docgen(); @@ -75,13 +76,14 @@ main(int argc, char** argv) } } - static GOptionEntry entries[] = { + GOptionEntry entries[] = { { "version", 'v', 0, G_OPTION_ARG_NONE, &version, "Show version information", NULL }, { "account", 'a', 0, G_OPTION_ARG_STRING, &account_name, "Auto connect to an account on startup" }, { "log", 'l', 0, G_OPTION_ARG_STRING, &log, "Set logging levels, DEBUG, INFO, WARN (default), ERROR", "LEVEL" }, { "config", 'c', 0, G_OPTION_ARG_STRING, &config_file, "Use an alternative configuration file", NULL }, { "logfile", 'f', 0, G_OPTION_ARG_STRING, &log_file, "Specify log file", NULL }, { "theme", 't', 0, G_OPTION_ARG_STRING, &theme_name, "Specify theme name", NULL }, + { "cmd", 'r', 0, G_OPTION_ARG_STRING_ARRAY, &commands, "First commands to run", NULL }, { NULL } }; @@ -171,14 +173,7 @@ main(int argc, char** argv) } /* Default logging WARN */ - prof_run(log ? log : "WARN", account_name, config_file, log_file, theme_name); - - /* Free resources allocated by GOptionContext */ - g_free(log); - g_free(account_name); - g_free(config_file); - g_free(log_file); - g_free(theme_name); + prof_run(log ? log : "WARN", account_name, config_file, log_file, theme_name, commands); return 0; } diff --git a/src/profanity.c b/src/profanity.c index c9e692920..dea5a6a10 100644 --- a/src/profanity.c +++ b/src/profanity.c @@ -34,6 +34,7 @@ * */ #include "config.h" +#include #ifdef HAVE_GTK #include "ui/tray.h" @@ -83,6 +84,8 @@ #include "omemo/omemo.h" #endif +static unsigned int min_runtime = 5; + static void _init(char* log_level, char* config_file, char* log_file, char* theme_name); static void _shutdown(void); static void _connect_default(const char* const account); @@ -91,7 +94,7 @@ pthread_mutex_t lock; static gboolean force_quit = FALSE; void -prof_run(char* log_level, char* account_name, char* config_file, char* log_file, char* theme_name) +prof_run(gchar* log_level, gchar* account_name, gchar* config_file, gchar* log_file, gchar* theme_name, gchar** commands) { gboolean cont = TRUE; @@ -105,17 +108,50 @@ prof_run(char* log_level, char* account_name, char* config_file, char* log_file, session_init_activity(); + GTimer* runtime = g_timer_new(); + char* line = NULL; + GTimer* waittimer = g_timer_new(); + g_timer_stop(waittimer); + int waittime; while (cont && !force_quit) { log_stderr_handler(); session_check_autoaway(); - line = inp_readline(); - if (line) { + line = commands ? *commands : inp_readline(); + if (commands && line && memcmp(line, "/sleep", 6) == 0) { + if (!g_timer_is_active(waittimer)) { + gchar* err_msg; + if (strtoi_range(line + 7, &waittime, 0, 300, &err_msg)) { + g_timer_start(waittimer); + /* Increase the minimal runtime by the waiting time + * so we can be sure there's runtime left after executing + * the last command. + */ + min_runtime += waittime; + } else { + log_error(err_msg); + g_free(err_msg); + commands = NULL; + } + } else if (g_timer_elapsed(waittimer, NULL) >= waittime) { + g_timer_stop(waittimer); + commands++; + if (!(*commands)) + commands = NULL; + } + cont = TRUE; + } else if (line) { ProfWin* window = wins_get_current(); cont = cmd_process_input(window, line); - free(line); - line = NULL; + if (commands) { + commands++; + if (!(*commands)) + commands = NULL; + } else { + free(line); + line = NULL; + } } else { cont = TRUE; } @@ -132,6 +168,9 @@ prof_run(char* log_level, char* account_name, char* config_file, char* log_file, tray_update(); #endif } + g_timer_destroy(waittimer); + g_timer_elapsed(runtime, NULL) < min_runtime ? sleep(min_runtime) : (void)NULL; + g_timer_destroy(runtime); } gboolean diff --git a/src/profanity.h b/src/profanity.h index 8a7d525a4..a7edc2cc2 100644 --- a/src/profanity.h +++ b/src/profanity.h @@ -40,7 +40,7 @@ #include #include -void prof_run(char* log_level, char* account_name, char* config_file, char* log_file, char* theme_name); +void prof_run(gchar* log_level, gchar* account_name, gchar* config_file, gchar* log_file, gchar* theme_name, gchar** commands); gboolean prof_set_quit(void); extern pthread_mutex_t lock; From c5a131ee466188e4b8e958752bdfeff3f0d598ad Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 7 Mar 2025 11:44:31 +0100 Subject: [PATCH 02/13] Introduce our own shutdown callback mechanism. Instead of adding stuff to `_shutdown()`, we can now register a shutdown routine from the respective `init()` function of our modules. This also has the advantage, that we're sure they're called in reverse order from how the initialization happened. I didn't simply use `atexit()` because POSIX says that the number of possible callbacks is limited (min 32) and I was not sure whether we will maybe extend this number at one point. Signed-off-by: Steffen Jaeckel --- src/chatlog.c | 30 ++++----- src/chatlog.h | 4 +- src/command/cmd_defs.c | 18 +++--- src/command/cmd_defs.h | 1 - src/common.h | 1 + src/config/accounts.c | 21 +++--- src/config/accounts.h | 1 - src/config/theme.c | 46 ++++++------- src/config/tlscerts.c | 27 ++++---- src/config/tlscerts.h | 2 - src/log.c | 32 ++++----- src/log.h | 1 - src/omemo/omemo.c | 23 ++++--- src/omemo/omemo.h | 1 - src/otr/otr.c | 22 ++++--- src/otr/otr.h | 1 - src/pgp/gpg.c | 42 ++++++------ src/pgp/gpg.h | 1 - src/plugins/plugins.c | 90 +++++++++++++------------- src/plugins/plugins.h | 2 - src/profanity.c | 71 ++++++++++++-------- src/ui/core.c | 31 ++++----- src/ui/tray.c | 23 +++---- src/ui/tray.h | 1 - src/ui/ui.h | 1 - src/xmpp/capabilities.c | 29 +++++---- src/xmpp/muc.c | 23 +++---- src/xmpp/muc.h | 1 - src/xmpp/session.c | 29 +++++---- src/xmpp/xmpp.h | 2 - tests/unittests/chatlog/stub_chatlog.c | 7 +- tests/unittests/config/stub_accounts.c | 4 -- tests/unittests/log/stub_log.c | 4 -- tests/unittests/omemo/stub_omemo.c | 4 -- tests/unittests/otr/stub_otr.c | 4 -- tests/unittests/pgp/stub_gpg.c | 4 -- tests/unittests/test_server_events.c | 11 ++-- tests/unittests/ui/stub_ui.c | 4 -- tests/unittests/xmpp/stub_xmpp.c | 9 --- 39 files changed, 311 insertions(+), 317 deletions(-) diff --git a/src/chatlog.c b/src/chatlog.c index 313ad71a8..8e18a6a7c 100644 --- a/src/chatlog.c +++ b/src/chatlog.c @@ -73,12 +73,25 @@ static void _chat_log_chat(const char* const login, const char* const other, con chat_log_direction_t direction, GDateTime* timestamp, const char* const resourcepart); static void _groupchat_log_chat(const gchar* const login, const gchar* const room, const gchar* const nick, const gchar* const msg); + +void +_chatlog_close(void) +{ + g_hash_table_destroy(logs); + g_hash_table_destroy(groupchat_logs); +} + void -chat_log_init(void) +chatlog_init(void) { log_info("Initialising chat logs"); + + prof_add_shutdown_routine(_chatlog_close); + logs = g_hash_table_new_full(g_str_hash, (GEqualFunc)_key_equals, free, (GDestroyNotify)_free_chat_log); + groupchat_logs = g_hash_table_new_full(g_str_hash, (GEqualFunc)_key_equals, free, + (GDestroyNotify)_free_chat_log); } void @@ -295,14 +308,6 @@ _chat_log_chat(const char* const login, const char* const other, const char* msg g_date_time_unref(timestamp); } -void -groupchat_log_init(void) -{ - log_info("Initialising groupchat logs"); - groupchat_logs = g_hash_table_new_full(g_str_hash, (GEqualFunc)_key_equals, free, - (GDestroyNotify)_free_chat_log); -} - void groupchat_log_msg_out(const gchar* const room, const gchar* const msg) { @@ -391,13 +396,6 @@ _groupchat_log_chat(const gchar* const login, const gchar* const room, const gch g_date_time_unref(dt_tmp); } -void -chat_log_close(void) -{ - g_hash_table_destroy(logs); - g_hash_table_destroy(groupchat_logs); -} - static char* _get_log_filename(const char* const other, const char* const login, GDateTime* dt, gboolean is_room) { diff --git a/src/chatlog.h b/src/chatlog.h index af61c8a55..94717fbf1 100644 --- a/src/chatlog.h +++ b/src/chatlog.h @@ -45,7 +45,7 @@ typedef enum { PROF_OUT_LOG } chat_log_direction_t; -void chat_log_init(void); +void chatlog_init(void); void chat_log_msg_out(const char* const barejid, const char* const msg, const char* resource); void chat_log_otr_msg_out(const char* const barejid, const char* const msg, const char* resource); @@ -57,8 +57,6 @@ void chat_log_otr_msg_in(ProfMessage* message); void chat_log_pgp_msg_in(ProfMessage* message); void chat_log_omemo_msg_in(ProfMessage* message); -void chat_log_close(void); - void groupchat_log_init(void); void groupchat_log_msg_out(const gchar* const room, const gchar* const msg); diff --git a/src/command/cmd_defs.c b/src/command/cmd_defs.c index 0253d4113..d432cd0bd 100644 --- a/src/command/cmd_defs.c +++ b/src/command/cmd_defs.c @@ -2840,6 +2840,14 @@ cmd_search_index_all(char* term) return results; } +static void +_cmd_uninit(void) +{ + cmd_ac_uninit(); + g_hash_table_destroy(commands); + g_hash_table_destroy(search_index); +} + /* * Initialise command autocompleter and history */ @@ -2848,6 +2856,8 @@ cmd_init(void) { log_info("Initialising commands"); + prof_add_shutdown_routine(_cmd_uninit); + cmd_ac_init(); search_index = g_hash_table_new_full(g_str_hash, g_str_equal, free, g_free); @@ -2878,14 +2888,6 @@ cmd_init(void) prefs_free_aliases(aliases); } -void -cmd_uninit(void) -{ - cmd_ac_uninit(); - g_hash_table_destroy(commands); - g_hash_table_destroy(search_index); -} - gboolean cmd_valid_tag(const char* const str) { diff --git a/src/command/cmd_defs.h b/src/command/cmd_defs.h index cb4cf4819..846e79ddc 100644 --- a/src/command/cmd_defs.h +++ b/src/command/cmd_defs.h @@ -42,7 +42,6 @@ #include "ui/ui.h" void cmd_init(void); -void cmd_uninit(void); Command* cmd_get(const char* const command); GList* cmd_get_ordered(const char* const tag); diff --git a/src/common.h b/src/common.h index 5996c5d53..6e32e6ea3 100644 --- a/src/common.h +++ b/src/common.h @@ -196,5 +196,6 @@ gchar* get_expanded_path(const char* path); char* basename_from_url(const char* url); gchar* prof_get_version(void); +void prof_add_shutdown_routine(void (*routine)(void)); #endif diff --git a/src/config/accounts.c b/src/config/accounts.c index 203e94748..e8ea84b20 100644 --- a/src/config/accounts.c +++ b/src/config/accounts.c @@ -61,10 +61,22 @@ static Autocomplete enabled_ac; static void _save_accounts(void); +static void +_accounts_close(void) +{ + autocomplete_free(all_ac); + autocomplete_free(enabled_ac); + free_keyfile(&accounts_prof_keyfile); + accounts = NULL; +} + void accounts_load(void) { log_info("Loading accounts"); + + prof_add_shutdown_routine(_accounts_close); + all_ac = autocomplete_new(); enabled_ac = autocomplete_new(); load_data_keyfile(&accounts_prof_keyfile, FILE_ACCOUNTS); @@ -82,15 +94,6 @@ accounts_load(void) } } -void -accounts_close(void) -{ - autocomplete_free(all_ac); - autocomplete_free(enabled_ac); - free_keyfile(&accounts_prof_keyfile); - accounts = NULL; -} - char* accounts_find_enabled(const char* const prefix, gboolean previous, void* context) { diff --git a/src/config/accounts.h b/src/config/accounts.h index 06599054d..d72ebac26 100644 --- a/src/config/accounts.h +++ b/src/config/accounts.h @@ -42,7 +42,6 @@ #include "config/account.h" void accounts_load(void); -void accounts_close(void); char* accounts_find_all(const char* const prefix, gboolean previous, void* context); char* accounts_find_enabled(const char* const prefix, gboolean previous, void* context); diff --git a/src/config/theme.c b/src/config/theme.c index 63940f125..7cb459dfc 100644 --- a/src/config/theme.c +++ b/src/config/theme.c @@ -66,6 +66,28 @@ static void _theme_list_dir(const gchar* const dir, GSList** result); static GString* _theme_find(const char* const theme_name); static gboolean _theme_load_file(const char* const theme_name); +static void +_theme_close(void) +{ + color_pair_cache_free(); + if (theme) { + g_key_file_free(theme); + theme = NULL; + } + if (theme_loc) { + g_string_free(theme_loc, TRUE); + theme_loc = NULL; + } + if (bold_items) { + g_hash_table_destroy(bold_items); + bold_items = NULL; + } + if (defaults) { + g_hash_table_destroy(defaults); + defaults = NULL; + } +} + void theme_init(const char* const theme_name) { @@ -77,6 +99,8 @@ theme_init(const char* const theme_name) } } + prof_add_shutdown_routine(_theme_close); + defaults = g_hash_table_new_full(g_str_hash, g_str_equal, free, free); // Set default colors @@ -248,28 +272,6 @@ theme_list(void) return result; } -void -theme_close(void) -{ - color_pair_cache_free(); - if (theme) { - g_key_file_free(theme); - theme = NULL; - } - if (theme_loc) { - g_string_free(theme_loc, TRUE); - theme_loc = NULL; - } - if (bold_items) { - g_hash_table_destroy(bold_items); - bold_items = NULL; - } - if (defaults) { - g_hash_table_destroy(defaults); - defaults = NULL; - } -} - void theme_init_colours(void) { diff --git a/src/config/tlscerts.c b/src/config/tlscerts.c index 35548b9f6..4f45da36a 100644 --- a/src/config/tlscerts.c +++ b/src/config/tlscerts.c @@ -56,10 +56,25 @@ static Autocomplete certs_ac; static char* current_fp; +static void +_tlscerts_close(void) +{ + free_keyfile(&tlscerts_prof_keyfile); + tlscerts = NULL; + + free(current_fp); + current_fp = NULL; + + autocomplete_free(certs_ac); +} + void tlscerts_init(void) { log_info("Loading TLS certificates"); + + prof_add_shutdown_routine(_tlscerts_close); + load_data_keyfile(&tlscerts_prof_keyfile, FILE_TLSCERTS); tlscerts = tlscerts_prof_keyfile.keyfile; @@ -356,18 +371,6 @@ tlscerts_free(TLSCertificate* cert) } } -void -tlscerts_close(void) -{ - free_keyfile(&tlscerts_prof_keyfile); - tlscerts = NULL; - - free(current_fp); - current_fp = NULL; - - autocomplete_free(certs_ac); -} - static void _save_tlscerts(void) { diff --git a/src/config/tlscerts.h b/src/config/tlscerts.h index ab3d00e27..b8c9b5c4c 100644 --- a/src/config/tlscerts.h +++ b/src/config/tlscerts.h @@ -96,6 +96,4 @@ char* tlscerts_complete(const char* const prefix, gboolean previous, void* conte void tlscerts_reset_ac(void); -void tlscerts_close(void); - #endif diff --git a/src/log.c b/src/log.c index 968b30e2c..cacf573bc 100644 --- a/src/log.c +++ b/src/log.c @@ -313,6 +313,21 @@ log_stderr_nonblock_set(int fd) return rc; } +static void +_log_stderr_close(void) +{ + if (!stderr_inited) + return; + + /* handle remaining logs before close */ + log_stderr_handler(); + stderr_inited = 0; + free(stderr_buf); + g_string_free(stderr_msg, TRUE); + close(stderr_pipe[0]); + close(stderr_pipe[1]); +} + void log_stderr_init(log_level_t level) { @@ -337,6 +352,8 @@ log_stderr_init(log_level_t level) stderr_level = level; stderr_inited = 1; + prof_add_shutdown_routine(_log_stderr_close); + if (stderr_buf == NULL || stderr_msg == NULL) { errno = ENOMEM; goto err_free; @@ -354,18 +371,3 @@ log_stderr_init(log_level_t level) stderr_inited = 0; log_error("Unable to init stderr log handler: %s", strerror(errno)); } - -void -log_stderr_close(void) -{ - if (!stderr_inited) - return; - - /* handle remaining logs before close */ - log_stderr_handler(); - stderr_inited = 0; - free(stderr_buf); - g_string_free(stderr_msg, TRUE); - close(stderr_pipe[0]); - close(stderr_pipe[1]); -} diff --git a/src/log.h b/src/log.h index 671747ae3..14394a243 100644 --- a/src/log.h +++ b/src/log.h @@ -59,7 +59,6 @@ int log_level_from_string(char* log_level, log_level_t* level); const char* log_string_from_level(log_level_t level); void log_stderr_init(log_level_t level); -void log_stderr_close(void); void log_stderr_handler(void); #endif diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index da0bfc531..5b512c19c 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -116,10 +116,23 @@ static struct omemo_static_data GHashTable* fingerprint_ac; } omemo_static_data; +static void +_omemo_close(void) +{ + if (omemo_static_data.fingerprint_ac) { + g_hash_table_destroy(omemo_static_data.fingerprint_ac); + omemo_static_data.fingerprint_ac = NULL; + } + pthread_mutex_destroy(&omemo_static_data.lock); +} + void omemo_init(void) { log_info("[OMEMO] initialising"); + + prof_add_shutdown_routine(_omemo_close); + if (omemo_crypto_init() != 0) { cons_show("Error initializing OMEMO crypto: gcry_check_version() failed"); } @@ -131,16 +144,6 @@ omemo_init(void) omemo_static_data.fingerprint_ac = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)autocomplete_free); } -void -omemo_close(void) -{ - if (omemo_static_data.fingerprint_ac) { - g_hash_table_destroy(omemo_static_data.fingerprint_ac); - omemo_static_data.fingerprint_ac = NULL; - } - pthread_mutex_destroy(&omemo_static_data.lock); -} - void omemo_on_connect(ProfAccount* account) { diff --git a/src/omemo/omemo.h b/src/omemo/omemo.h index 040ab4b75..39479da5f 100644 --- a/src/omemo/omemo.h +++ b/src/omemo/omemo.h @@ -61,7 +61,6 @@ typedef struct omemo_key } omemo_key_t; void omemo_init(void); -void omemo_close(void); void omemo_on_connect(ProfAccount* account); void omemo_on_disconnect(void); void omemo_generate_crypto_materials(ProfAccount* account); diff --git a/src/otr/otr.c b/src/otr/otr.c index cd774f56e..1ec5695d3 100644 --- a/src/otr/otr.c +++ b/src/otr/otr.c @@ -166,12 +166,24 @@ otr_start_query(void) return otrlib_start_query(); } +static void +_otr_shutdown(void) +{ + if (jid) { + free(jid); + jid = NULL; + } + otrlib_shutdown(); +} + void otr_init(void) { log_info("Initialising OTR"); OTRL_INIT; + prof_add_shutdown_routine(_otr_shutdown); + jid = NULL; ops.policy = cb_policy; @@ -187,16 +199,6 @@ otr_init(void) data_loaded = FALSE; } -void -otr_shutdown(void) -{ - if (jid) { - free(jid); - jid = NULL; - } - otrlib_shutdown(); -} - void otr_poll(void) { diff --git a/src/otr/otr.h b/src/otr/otr.h index c909c7212..ee5255de6 100644 --- a/src/otr/otr.h +++ b/src/otr/otr.h @@ -66,7 +66,6 @@ OtrlMessageAppOps* otr_messageops(void); GHashTable* otr_smpinitators(void); void otr_init(void); -void otr_shutdown(void); char* otr_libotr_version(void); char* otr_start_query(void); void otr_poll(void); diff --git a/src/pgp/gpg.c b/src/pgp/gpg.c index 8cb903624..483ef8c33 100644 --- a/src/pgp/gpg.c +++ b/src/pgp/gpg.c @@ -111,25 +111,8 @@ _p_gpg_passphrase_cb(void* hook, const char* uid_hint, const char* passphrase_in return 0; } -void -p_gpg_init(void) -{ - libversion = gpgme_check_version(NULL); - log_debug("GPG: Found gpgme version: %s", libversion); - gpgme_set_locale(NULL, LC_CTYPE, setlocale(LC_CTYPE, NULL)); - - pubkeys = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, (GDestroyNotify)_p_gpg_free_pubkeyid); - - key_ac = autocomplete_new(); - GHashTable* keys = p_gpg_list_keys(); - p_gpg_free_keys(keys); - - passphrase = NULL; - passphrase_attempt = NULL; -} - -void -p_gpg_close(void) +static void +_p_gpg_close(void) { if (pubkeys) { g_hash_table_destroy(pubkeys); @@ -153,6 +136,25 @@ p_gpg_close(void) } } +void +p_gpg_init(void) +{ + libversion = gpgme_check_version(NULL); + log_debug("GPG: Found gpgme version: %s", libversion); + gpgme_set_locale(NULL, LC_CTYPE, setlocale(LC_CTYPE, NULL)); + + prof_add_shutdown_routine(_p_gpg_close); + + pubkeys = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, (GDestroyNotify)_p_gpg_free_pubkeyid); + + key_ac = autocomplete_new(); + GHashTable* keys = p_gpg_list_keys(); + p_gpg_free_keys(keys); + + passphrase = NULL; + passphrase_attempt = NULL; +} + void p_gpg_on_connect(const char* const barejid) { @@ -208,7 +210,7 @@ p_gpg_on_connect(const char* const barejid) void p_gpg_on_disconnect(void) { - p_gpg_close(); + _p_gpg_close(); pubkeys = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, (GDestroyNotify)_p_gpg_free_pubkeyid); key_ac = autocomplete_new(); } diff --git a/src/pgp/gpg.h b/src/pgp/gpg.h index 883aaf734..d1cff52e9 100644 --- a/src/pgp/gpg.h +++ b/src/pgp/gpg.h @@ -55,7 +55,6 @@ typedef struct pgp_pubkeyid_t } ProfPGPPubKeyId; void p_gpg_init(void); -void p_gpg_close(void); void p_gpg_on_connect(const char* const barejid); void p_gpg_on_disconnect(void); GHashTable* p_gpg_list_keys(void); diff --git a/src/plugins/plugins.c b/src/plugins/plugins.c index 6238525d4..2236e94fd 100644 --- a/src/plugins/plugins.c +++ b/src/plugins/plugins.c @@ -65,9 +65,49 @@ static GHashTable* plugins; +static void +_plugins_shutdown(void) +{ + GList* values = g_hash_table_get_values(plugins); + GList *curr = values, *next; + + while (curr) { + next = g_list_next(curr); +#ifdef HAVE_PYTHON + if (curr && ((ProfPlugin*)curr->data)->lang == LANG_PYTHON) { + python_plugin_destroy(curr->data); + curr = NULL; + } +#endif +#ifdef HAVE_C + if (curr && ((ProfPlugin*)curr->data)->lang == LANG_C) { + c_plugin_destroy(curr->data); + curr = NULL; + } +#endif + curr = next; + } + g_list_free(values); +#ifdef HAVE_PYTHON + python_shutdown(); +#endif +#ifdef HAVE_C + c_shutdown(); +#endif + + autocompleters_destroy(); + plugin_themes_close(); + plugin_settings_close(); + callbacks_close(); + disco_close(); + g_hash_table_destroy(plugins); + plugins = NULL; +} + void plugins_init(void) { + prof_add_shutdown_routine(_plugins_shutdown); plugins = g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL); callbacks_init(); autocompleters_init(); @@ -421,27 +461,28 @@ plugins_close_win(const char* const plugin_name, const char* const tag) callbacks_remove_win(plugin_name, tag); } -void -plugins_on_start(void) +static void +_plugins_on_shutdown(void) { GList* values = g_hash_table_get_values(plugins); GList* curr = values; while (curr) { ProfPlugin* plugin = curr->data; - plugin->on_start_func(plugin); + plugin->on_shutdown_func(plugin); curr = g_list_next(curr); } g_list_free(values); } void -plugins_on_shutdown(void) +plugins_on_start(void) { + prof_add_shutdown_routine(_plugins_on_shutdown); GList* values = g_hash_table_get_values(plugins); GList* curr = values; while (curr) { ProfPlugin* plugin = curr->data; - plugin->on_shutdown_func(plugin); + plugin->on_start_func(plugin); curr = g_list_next(curr); } g_list_free(values); @@ -924,42 +965,3 @@ plugins_get_disco_features(void) { return disco_get_features(); } - -void -plugins_shutdown(void) -{ - GList* values = g_hash_table_get_values(plugins); - GList *curr = values, *next; - - while (curr) { - next = g_list_next(curr); -#ifdef HAVE_PYTHON - if (curr && ((ProfPlugin*)curr->data)->lang == LANG_PYTHON) { - python_plugin_destroy(curr->data); - curr = NULL; - } -#endif -#ifdef HAVE_C - if (curr && ((ProfPlugin*)curr->data)->lang == LANG_C) { - c_plugin_destroy(curr->data); - curr = NULL; - } -#endif - curr = next; - } - g_list_free(values); -#ifdef HAVE_PYTHON - python_shutdown(); -#endif -#ifdef HAVE_C - c_shutdown(); -#endif - - autocompleters_destroy(); - plugin_themes_close(); - plugin_settings_close(); - callbacks_close(); - disco_close(); - g_hash_table_destroy(plugins); - plugins = NULL; -} diff --git a/src/plugins/plugins.h b/src/plugins/plugins.h index 0bd69e41e..85087e51d 100644 --- a/src/plugins/plugins.h +++ b/src/plugins/plugins.h @@ -113,7 +113,6 @@ GSList* plugins_unloaded_list(void); GList* plugins_loaded_list(void); char* plugins_autocomplete(const char* const input, gboolean previous); void plugins_reset_autocomplete(void); -void plugins_shutdown(void); void plugins_free_install_result(PluginsInstallResult* result); @@ -129,7 +128,6 @@ gboolean plugins_reload(const char* const name, GString* error_message); void plugins_reload_all(void); void plugins_on_start(void); -void plugins_on_shutdown(void); void plugins_on_connect(const char* const account_name, const char* const fulljid); void plugins_on_disconnect(const char* const account_name, const char* const fulljid); diff --git a/src/profanity.c b/src/profanity.c index dea5a6a10..0b639bb16 100644 --- a/src/profanity.c +++ b/src/profanity.c @@ -220,8 +220,7 @@ _init(char* log_level, char* config_file, char* log_file, char* theme_name) auto_gchar gchar* prof_version = prof_get_version(); log_info("Starting Profanity (%s)…", prof_version); - chat_log_init(); - groupchat_log_init(); + chatlog_init(); accounts_load(); if (theme_name) { @@ -261,6 +260,47 @@ _init(char* log_level, char* config_file, char* log_file, char* theme_name) ui_resize(); } +static GList* shutdown_routines = NULL; + +/* We have to encapsulate the function pointer, since the C standard does not guarantee + * that a void* can be converted to a function* and vice-versa. + */ +struct shutdown_routine +{ + void (*routine)(void); +}; + +static gint +_cmp_shutdown_routine(const struct shutdown_routine* a, const struct shutdown_routine* b) +{ + return a->routine != b->routine; +} + +void +prof_add_shutdown_routine(void (*routine)(void)) +{ + struct shutdown_routine this = { .routine = routine }; + if (g_list_find_custom(shutdown_routines, &this, (GCompareFunc)_cmp_shutdown_routine)) { + return; + } + struct shutdown_routine* r = malloc(sizeof *r); + r->routine = routine; + shutdown_routines = g_list_prepend(shutdown_routines, r); +} + +static void +_call_and_free_shutdown_routine(struct shutdown_routine* r) +{ + r->routine(); + free(r); +} + +void +prof_shutdown(void) +{ + g_list_free_full(shutdown_routines, (GDestroyNotify)_call_and_free_shutdown_routine); +} + static void _shutdown(void) { @@ -276,30 +316,9 @@ _shutdown(void) if (conn_status == JABBER_CONNECTED) { cl_ev_disconnect(); } -#ifdef HAVE_GTK - tray_shutdown(); -#endif - session_shutdown(); - plugins_on_shutdown(); - muc_close(); - caps_close(); -#ifdef HAVE_LIBOTR - otr_shutdown(); -#endif -#ifdef HAVE_LIBGPGME - p_gpg_close(); -#endif -#ifdef HAVE_OMEMO - omemo_close(); -#endif - chat_log_close(); - theme_close(); - accounts_close(); - tlscerts_close(); - log_stderr_close(); - plugins_shutdown(); - cmd_uninit(); - ui_close(); + prof_shutdown(); + /* Prefs and logs have to be closed in swapped order, so they're no using the automatic + * shutdown mechanism. */ prefs_close(); log_close(); } diff --git a/src/ui/core.c b/src/ui/core.c index 5db891374..b5a2eb1ab 100644 --- a/src/ui/core.c +++ b/src/ui/core.c @@ -89,10 +89,26 @@ static Display* display; static void _ui_draw_term_title(void); +static void +_ui_close(void) +{ + g_timer_destroy(ui_idle_time); + endwin(); + notifier_uninit(); + cons_clear_alerts(); + wins_destroy(); + inp_close(); + status_bar_close(); + free_title_bar(); + delwin(main_scr); + delscreen(set_term(NULL)); +} + void ui_init(void) { log_info("Initialising UI"); + prof_add_shutdown_routine(_ui_close); main_scr = initscr(); nonl(); cbreak(); @@ -180,21 +196,6 @@ ui_reset_idle_time(void) g_timer_start(ui_idle_time); } -void -ui_close(void) -{ - g_timer_destroy(ui_idle_time); - endwin(); - notifier_uninit(); - cons_clear_alerts(); - wins_destroy(); - inp_close(); - status_bar_close(); - free_title_bar(); - delwin(main_scr); - delscreen(set_term(NULL)); -} - void ui_resize(void) { diff --git a/src/ui/tray.c b/src/ui/tray.c index 0e577f8ed..a1b286926 100644 --- a/src/ui/tray.c +++ b/src/ui/tray.c @@ -153,9 +153,21 @@ _tray_change_icon(gpointer data) return TRUE; } +static void +_tray_shutdown(void) +{ + if (gtk_ready && prefs_get_boolean(PREF_TRAY)) { + tray_disable(); + } + g_string_free(icon_filename, TRUE); + g_string_free(icon_msg_filename, TRUE); + gtk_main_quit(); +} + void tray_init(void) { + prof_add_shutdown_routine(_tray_shutdown); _get_icons(); gtk_ready = gtk_init_check(0, NULL); log_debug("Env is GTK-ready: %s", gtk_ready ? "true" : "false"); @@ -179,17 +191,6 @@ tray_update(void) } } -void -tray_shutdown(void) -{ - if (gtk_ready && prefs_get_boolean(PREF_TRAY)) { - tray_disable(); - } - g_string_free(icon_filename, TRUE); - g_string_free(icon_msg_filename, TRUE); - gtk_main_quit(); -} - void tray_set_timer(int interval) { diff --git a/src/ui/tray.h b/src/ui/tray.h index 89e51ec00..bb4491257 100644 --- a/src/ui/tray.h +++ b/src/ui/tray.h @@ -39,7 +39,6 @@ #ifdef HAVE_GTK void tray_init(void); void tray_update(void); -void tray_shutdown(void); void tray_enable(void); void tray_disable(void); diff --git a/src/ui/ui.h b/src/ui/ui.h index 8841ba07e..a8d04ab04 100644 --- a/src/ui/ui.h +++ b/src/ui/ui.h @@ -62,7 +62,6 @@ void ui_init(void); void ui_load_colours(void); void ui_update(void); -void ui_close(void); void ui_redraw(void); void ui_resize(void); void ui_focus_win(ProfWin* window); diff --git a/src/xmpp/capabilities.c b/src/xmpp/capabilities.c index 96bbd5746..4e1e5e2c5 100644 --- a/src/xmpp/capabilities.c +++ b/src/xmpp/capabilities.c @@ -69,10 +69,26 @@ static EntityCapabilities* _caps_by_ver(const char* const ver); static EntityCapabilities* _caps_by_jid(const char* const jid); static EntityCapabilities* _caps_copy(EntityCapabilities* caps); +static void +_caps_close(void) +{ + free_keyfile(&caps_prof_keyfile); + cache = NULL; + g_hash_table_destroy(jid_to_ver); + g_hash_table_destroy(jid_to_caps); + g_free(cache_loc); + cache_loc = NULL; + g_hash_table_destroy(prof_features); + prof_features = NULL; +} + void caps_init(void) { log_info("Loading capabilities cache"); + + prof_add_shutdown_routine(_caps_close); + load_data_keyfile(&caps_prof_keyfile, FILE_CAPSCACHE); cache = caps_prof_keyfile.keyfile; @@ -339,19 +355,6 @@ caps_reset_ver(void) } } -void -caps_close(void) -{ - free_keyfile(&caps_prof_keyfile); - cache = NULL; - g_hash_table_destroy(jid_to_ver); - g_hash_table_destroy(jid_to_caps); - g_free(cache_loc); - cache_loc = NULL; - g_hash_table_destroy(prof_features); - prof_features = NULL; -} - static EntityCapabilities* _caps_by_ver(const char* const ver) { diff --git a/src/xmpp/muc.c b/src/xmpp/muc.c index 5c47cd752..b6bb57c9d 100644 --- a/src/xmpp/muc.c +++ b/src/xmpp/muc.c @@ -91,17 +91,8 @@ static Occupant* _muc_occupant_new(const char* const nick, const char* const jid muc_affiliation_t affiliation, resource_presence_t presence, const char* const status); static void _occupant_free(Occupant* occupant); -void -muc_init(void) -{ - invite_ac = autocomplete_new(); - confservers_ac = autocomplete_new(); - rooms = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, (GDestroyNotify)_free_room); - invite_passwords = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, g_free); -} - -void -muc_close(void) +static void +_muc_close(void) { autocomplete_free(invite_ac); autocomplete_free(confservers_ac); @@ -113,6 +104,16 @@ muc_close(void) confservers_ac = NULL; } +void +muc_init(void) +{ + prof_add_shutdown_routine(_muc_close); + invite_ac = autocomplete_new(); + confservers_ac = autocomplete_new(); + rooms = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, (GDestroyNotify)_free_room); + invite_passwords = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, g_free); +} + void muc_confserver_add(const char* const server) { diff --git a/src/xmpp/muc.h b/src/xmpp/muc.h index 5e099ceaf..7ae7a0c9d 100644 --- a/src/xmpp/muc.h +++ b/src/xmpp/muc.h @@ -82,7 +82,6 @@ typedef struct _muc_occupant_t } Occupant; void muc_init(void); -void muc_close(void); void muc_join(const char* const room, const char* const nick, const char* const password, gboolean autojoin); void muc_leave(const char* const room); diff --git a/src/xmpp/session.c b/src/xmpp/session.c index e1b4c1a3a..8716bb0d9 100644 --- a/src/xmpp/session.c +++ b/src/xmpp/session.c @@ -99,10 +99,25 @@ static char* saved_status; static void _session_free_internals(void); static void _session_free_saved_details(void); +static void +_session_shutdown(void) +{ + _session_free_internals(); + + chat_sessions_clear(); + presence_sub_requests_destroy(); + + connection_shutdown(); + if (saved_status) { + free(saved_status); + } +} + void session_init(void) { log_info("Initialising XMPP"); + prof_add_shutdown_routine(_session_shutdown); connection_init(); presence_sub_requests_init(); caps_init(); @@ -227,20 +242,6 @@ session_disconnect(void) connection_set_disconnected(); } -void -session_shutdown(void) -{ - _session_free_internals(); - - chat_sessions_clear(); - presence_sub_requests_destroy(); - - connection_shutdown(); - if (saved_status) { - free(saved_status); - } -} - void session_process_events(void) { diff --git a/src/xmpp/xmpp.h b/src/xmpp/xmpp.h index 755ad2733..f6ed92ddd 100644 --- a/src/xmpp/xmpp.h +++ b/src/xmpp/xmpp.h @@ -183,7 +183,6 @@ jabber_conn_status_t session_connect_with_details(const char* const jid, const c jabber_conn_status_t session_connect_with_account(const ProfAccount* const account); void session_disconnect(void); -void session_shutdown(void); void session_process_events(void); const char* session_get_account_name(void); void session_reconnect_now(void); @@ -275,7 +274,6 @@ void iq_muc_register_nick(const char* const roomjid); void autoping_timer_extend(void); EntityCapabilities* caps_lookup(const char* const jid); -void caps_close(void); void caps_destroy(EntityCapabilities* caps); void caps_reset_ver(void); void caps_add_feature(char* feature); diff --git a/tests/unittests/chatlog/stub_chatlog.c b/tests/unittests/chatlog/stub_chatlog.c index 33251f579..2d8eb382e 100644 --- a/tests/unittests/chatlog/stub_chatlog.c +++ b/tests/unittests/chatlog/stub_chatlog.c @@ -27,7 +27,7 @@ #include void -chat_log_init(void) +chatlog_init(void) { } @@ -65,11 +65,6 @@ chat_log_omemo_msg_in(ProfMessage* message) { } -void -chat_log_close(void) -{ -} - void groupchat_log_init(void) { diff --git a/tests/unittests/config/stub_accounts.c b/tests/unittests/config/stub_accounts.c index a3fcacc76..360e56431 100644 --- a/tests/unittests/config/stub_accounts.c +++ b/tests/unittests/config/stub_accounts.c @@ -10,10 +10,6 @@ void accounts_load(void) { } -void -accounts_close(void) -{ -} char* accounts_find_all(const char* const prefix, gboolean previous, void* context) diff --git a/tests/unittests/log/stub_log.c b/tests/unittests/log/stub_log.c index 18d05eca0..23ab5cdc8 100644 --- a/tests/unittests/log/stub_log.c +++ b/tests/unittests/log/stub_log.c @@ -78,10 +78,6 @@ log_stderr_init(log_level_t level) { } void -log_stderr_close(void) -{ -} -void log_stderr_handler(void) { } diff --git a/tests/unittests/omemo/stub_omemo.c b/tests/unittests/omemo/stub_omemo.c index 0336e1a38..986de0793 100644 --- a/tests/unittests/omemo/stub_omemo.c +++ b/tests/unittests/omemo/stub_omemo.c @@ -7,10 +7,6 @@ void omemo_init(void) { } -void -omemo_close(void) -{ -} char* omemo_fingerprint_autocomplete(const char* const search_str, gboolean previous) diff --git a/tests/unittests/otr/stub_otr.c b/tests/unittests/otr/stub_otr.c index d6827c666..291db41c2 100644 --- a/tests/unittests/otr/stub_otr.c +++ b/tests/unittests/otr/stub_otr.c @@ -33,10 +33,6 @@ void otr_init(void) { } -void -otr_shutdown(void) -{ -} char* otr_libotr_version(void) diff --git a/tests/unittests/pgp/stub_gpg.c b/tests/unittests/pgp/stub_gpg.c index 6333e7234..249a2c9c0 100644 --- a/tests/unittests/pgp/stub_gpg.c +++ b/tests/unittests/pgp/stub_gpg.c @@ -6,10 +6,6 @@ void p_gpg_init(void) { } -void -p_gpg_close(void) -{ -} GHashTable* p_gpg_list_keys(void) diff --git a/tests/unittests/test_server_events.c b/tests/unittests/test_server_events.c index 4ecedace0..feed7709d 100644 --- a/tests/unittests/test_server_events.c +++ b/tests/unittests/test_server_events.c @@ -17,6 +17,9 @@ #include "plugins/plugins.h" #include "ui/window_list.h" + +void prof_shutdown(void); + void console_shows_online_presence_when_set_online(void** state) { @@ -35,7 +38,7 @@ console_shows_online_presence_when_set_online(void** state) sv_ev_contact_online(barejid, resource, NULL, NULL); roster_destroy(); - plugins_shutdown(); + prof_shutdown(); } void @@ -56,7 +59,7 @@ console_shows_online_presence_when_set_all(void** state) sv_ev_contact_online(barejid, resource, NULL, NULL); roster_destroy(); - plugins_shutdown(); + prof_shutdown(); } void @@ -77,7 +80,7 @@ console_shows_dnd_presence_when_set_all(void** state) sv_ev_contact_online(barejid, resource, NULL, NULL); roster_destroy(); - plugins_shutdown(); + prof_shutdown(); } void @@ -103,7 +106,7 @@ handle_offline_removes_chat_session(void** state) roster_destroy(); chat_sessions_clear(); - plugins_shutdown(); + prof_shutdown(); } void diff --git a/tests/unittests/ui/stub_ui.c b/tests/unittests/ui/stub_ui.c index f45f6b070..4c6439dd0 100644 --- a/tests/unittests/ui/stub_ui.c +++ b/tests/unittests/ui/stub_ui.c @@ -63,10 +63,6 @@ ui_update(void) { } void -ui_close(void) -{ -} -void ui_redraw(void) { } diff --git a/tests/unittests/xmpp/stub_xmpp.c b/tests/unittests/xmpp/stub_xmpp.c index 1ff25e69a..366f26b39 100644 --- a/tests/unittests/xmpp/stub_xmpp.c +++ b/tests/unittests/xmpp/stub_xmpp.c @@ -47,10 +47,6 @@ session_disconnect(void) { } void -session_shutdown(void) -{ -} -void session_process_events(void) { } @@ -477,11 +473,6 @@ caps_lookup(const char* const jid) return NULL; } -void -caps_close(void) -{ -} - void caps_destroy(EntityCapabilities* caps) { From 93fc4ecd8b7c7f923863e3819534d8b8760fd9dd Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 7 Mar 2025 11:47:02 +0100 Subject: [PATCH 03/13] Add some more false-positives to our Valgrind suppression list I hope I didn't overshoot :) Signed-off-by: Steffen Jaeckel --- prof.supp | 151 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 150 insertions(+), 1 deletion(-) diff --git a/prof.supp b/prof.supp index 4a218adf6..3d0c58804 100644 --- a/prof.supp +++ b/prof.supp @@ -15,6 +15,37 @@ fun:_dl_init ... } +{ + libfreetype + Memcheck:Leak + ... + obj:*/libfreetype* + ... + obj:*/libpango* + ... +} +{ + libfontconfig + Memcheck:Leak + ... + obj:*/libfontconfig* +} +{ + libncurses + Memcheck:Leak + ... + obj:*/libncurses* + ... +} +{ + otrl_init + Memcheck:Leak + ... + fun:otrl_init + ... +} + +# gtk { gtk_init_check @@ -30,8 +61,31 @@ fun:gtk_module_init ... } +{ + gtk_widget_get_preferred_height + Memcheck:Leak + ... + fun:gtk_widget_get_preferred_height + ... +} +{ + gtk_main_iteration_do + Memcheck:Leak + ... + fun:gtk_main_iteration_do + ... +} +{ + tray_enable + Memcheck:Leak + ... + fun:g_object_new + ... + fun:tray_enable + ... +} -# glib +# glib/gtk { glib @@ -40,6 +94,94 @@ fun:start_thread fun:clone } +{ + g_date_time_new_now_ variants + Memcheck:Leak + ... + fun:g_time_zone_new_identifier + ... + fun:g_date_time_new_now_* + ... +} +{ + g_date_time_new_from_ variants + Memcheck:Leak + ... + fun:g_time_zone_new_identifier + ... + fun:g_date_time_new_from_* + ... +} +{ + g_strerror/g_hash_table_new_full + Memcheck:Leak + ... + fun:g_hash_table_new_full + fun:g_strerror + ... +} +{ + g_strerror/g_hash_table_insert + Memcheck:Leak + ... + fun:g_hash_table_insert + fun:g_strerror + ... +} +{ + g_intern_string + Memcheck:Leak + ... + fun:g_intern_string + ... +} +{ + g_rw_lock_reader_lock + Memcheck:Leak + ... + fun:g_rw_lock_reader_lock + ... +} +{ + g_type_create_instance + Memcheck:Leak + ... + fun:g_type_create_instance +} +{ + g_object_new + Memcheck:Leak + ... + fun:g_object_new +} +{ + libgtk + Memcheck:Leak + ... + obj:*/libgtk* +} +{ + libgdk + Memcheck:Leak + ... + obj:*/libgdk* +} +{ + libgobject + Memcheck:Leak + ... + obj:*/libgobject* +} + +# fontconfig + +{ + FcConfigSubstituteWithPat + Memcheck:Leak + ... + fun:FcConfigSubstituteWithPat + ... +} # gcrypt initialization { @@ -103,6 +245,13 @@ ... fun:PyType_Ready } +{ + Handle PyMalloc confusing valgrind (possibly leaked) + Memcheck:Leak + ... + fun:Py_InitializeEx + fun:python_env_init +} # AUTO-GENERATED START From b55bf6e4bddff64b54564faf4bfb919c506a9ed0 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 7 Mar 2025 11:49:08 +0100 Subject: [PATCH 04/13] Fix some more memory leaks + an invalid read. `ProfMessage` was not completely initialized, fix this by using `calloc()` for the allocation. Signed-off-by: Steffen Jaeckel --- src/command/cmd_ac.c | 1 + src/otr/otr.c | 1 + src/xmpp/blocking.c | 10 ++++++++++ src/xmpp/bookmark.c | 10 ++++++++++ src/xmpp/capabilities.c | 1 + src/xmpp/message.c | 14 ++------------ 6 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/command/cmd_ac.c b/src/command/cmd_ac.c index 3681eed3f..3a4a46059 100644 --- a/src/command/cmd_ac.c +++ b/src/command/cmd_ac.c @@ -1647,6 +1647,7 @@ cmd_ac_uninit(void) autocomplete_free(plugins_unload_ac); autocomplete_free(plugins_reload_ac); autocomplete_free(script_show_ac); + g_hash_table_destroy(ac_funcs); } static void diff --git a/src/otr/otr.c b/src/otr/otr.c index 1ec5695d3..5e99575a3 100644 --- a/src/otr/otr.c +++ b/src/otr/otr.c @@ -169,6 +169,7 @@ otr_start_query(void) static void _otr_shutdown(void) { + g_hash_table_destroy(smp_initiators); if (jid) { free(jid); jid = NULL; diff --git a/src/xmpp/blocking.c b/src/xmpp/blocking.c index 0e8c215fd..f2a49c7d2 100644 --- a/src/xmpp/blocking.c +++ b/src/xmpp/blocking.c @@ -57,9 +57,19 @@ static int _block_remove_result_handler(xmpp_stanza_t* const stanza, void* const static GList* blocked; static Autocomplete blocked_ac; +static void +_blocking_shutdown(void) +{ + if (blocked) { + g_list_free_full(blocked, free); + } + autocomplete_free(blocked_ac); +} + void blocking_request(void) { + prof_add_shutdown_routine(_blocking_shutdown); if (blocked) { g_list_free_full(blocked, free); blocked = NULL; diff --git a/src/xmpp/bookmark.c b/src/xmpp/bookmark.c index 8721ff509..0e6a23b34 100644 --- a/src/xmpp/bookmark.c +++ b/src/xmpp/bookmark.c @@ -66,9 +66,19 @@ static int _bookmark_result_id_handler(xmpp_stanza_t* const stanza, void* const static void _bookmark_destroy(Bookmark* bookmark); static void _send_bookmarks(void); +static void +_bookmark_shutdown(void) +{ + if (bookmarks) { + g_hash_table_destroy(bookmarks); + } + autocomplete_free(bookmark_ac); +} + void bookmark_request(void) { + prof_add_shutdown_routine(_bookmark_shutdown); if (bookmarks) { g_hash_table_destroy(bookmarks); } diff --git a/src/xmpp/capabilities.c b/src/xmpp/capabilities.c index 4e1e5e2c5..8f1cf6d6b 100644 --- a/src/xmpp/capabilities.c +++ b/src/xmpp/capabilities.c @@ -72,6 +72,7 @@ static EntityCapabilities* _caps_copy(EntityCapabilities* caps); static void _caps_close(void) { + caps_reset_ver(); free_keyfile(&caps_prof_keyfile); cache = NULL; g_hash_table_destroy(jid_to_ver); diff --git a/src/xmpp/message.c b/src/xmpp/message.c index 388f5bb6b..3bf1dc0dc 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -347,19 +347,9 @@ message_handlers_init(void) ProfMessage* message_init(void) { - ProfMessage* message = malloc(sizeof(ProfMessage)); - - message->from_jid = NULL; - message->to_jid = NULL; - message->id = NULL; - message->originid = NULL; - message->stanzaid = NULL; - message->replace_id = NULL; - message->body = NULL; - message->encrypted = NULL; - message->plain = NULL; + ProfMessage* message = calloc(1, sizeof(ProfMessage)); + message->enc = PROF_MSG_ENC_NONE; - message->timestamp = NULL; message->trusted = true; message->type = PROF_MSG_TYPE_UNINITIALIZED; From 72b99ceb6dedb747c44fdd85d0d15a35bf12d730 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 7 Mar 2025 11:51:30 +0100 Subject: [PATCH 05/13] Some minor improvements * destroy/free/shutdown/close in reverse order of allocation * more static/auto_Xfree * less variables/strlen/GString * properly `\0`-terminate string of testcase Signed-off-by: Steffen Jaeckel --- src/plugins/callbacks.c | 4 ++-- src/plugins/plugins.c | 6 +++--- src/plugins/python_plugins.c | 11 ++++------- src/tools/http_upload.c | 20 ++++++-------------- src/ui/console.c | 3 +-- src/ui/core.c | 2 +- src/xmpp/chat_session.c | 2 +- src/xmpp/iq.c | 3 ++- tests/unittests/test_common.c | 2 +- 9 files changed, 21 insertions(+), 32 deletions(-) diff --git a/src/plugins/callbacks.c b/src/plugins/callbacks.c index ee49dc2af..366070663 100644 --- a/src/plugins/callbacks.c +++ b/src/plugins/callbacks.c @@ -177,9 +177,9 @@ callbacks_remove(const char* const plugin_name) void callbacks_close(void) { - g_hash_table_destroy(p_commands); - g_hash_table_destroy(p_timed_functions); g_hash_table_destroy(p_window_callbacks); + g_hash_table_destroy(p_timed_functions); + g_hash_table_destroy(p_commands); } void diff --git a/src/plugins/plugins.c b/src/plugins/plugins.c index 2236e94fd..7d43f3f41 100644 --- a/src/plugins/plugins.c +++ b/src/plugins/plugins.c @@ -95,9 +95,9 @@ _plugins_shutdown(void) c_shutdown(); #endif - autocompleters_destroy(); - plugin_themes_close(); plugin_settings_close(); + plugin_themes_close(); + autocompleters_destroy(); callbacks_close(); disco_close(); g_hash_table_destroy(plugins); @@ -399,7 +399,7 @@ plugins_reload(const char* const name, GString* error_message) return res; } -void +static void _plugins_unloaded_list_dir(const gchar* const dir, GSList** result) { GDir* plugins_dir = g_dir_open(dir, 0, NULL); diff --git a/src/plugins/python_plugins.c b/src/plugins/python_plugins.c index c999afa33..c25bb541e 100644 --- a/src/plugins/python_plugins.c +++ b/src/plugins/python_plugins.c @@ -100,16 +100,13 @@ python_env_init(void) python_init_prof(); auto_gchar gchar* plugins_dir = files_get_data_path(DIR_PLUGINS); - GString* path = g_string_new("import sys\n"); - g_string_append(path, "sys.path.append(\""); - g_string_append(path, plugins_dir); - g_string_append(path, "/\")\n"); + auto_gchar gchar* path = g_strdup_printf( + "import sys\n" + "sys.path.append(\"%s/\")\n", plugins_dir); - PyRun_SimpleString(path->str); + PyRun_SimpleString(path); python_check_error(); - g_string_free(path, TRUE); - allow_python_threads(); } diff --git a/src/tools/http_upload.c b/src/tools/http_upload.c index 3b3945f3e..2d0510237 100644 --- a/src/tools/http_upload.c +++ b/src/tools/http_upload.c @@ -164,11 +164,6 @@ http_file_put(void* userdata) FILE* fh = NULL; auto_char char* err = NULL; - gchar* content_type_header; - // Optional headers - gchar* auth_header = NULL; - gchar* cookie_header = NULL; - gchar* expires_header = NULL; CURL* curl; CURLcode res; @@ -177,15 +172,14 @@ http_file_put(void* userdata) upload->bytes_sent = 0; pthread_mutex_lock(&lock); - gchar* msg = g_strdup_printf("Uploading '%s': 0%%", upload->filename); + auto_gchar gchar* msg = g_strdup_printf("Uploading '%s': 0%%", upload->filename); if (!msg) { msg = g_strdup(FALLBACK_MSG); } win_print_http_transfer(upload->window, msg, upload->put_url); - g_free(msg); auto_gchar gchar* cert_path = prefs_get_string(PREF_TLS_CERTPATH); - gchar* cafile = cafile_get_name(); + auto_gchar gchar* cafile = cafile_get_name(); gboolean insecure = FALSE; ProfAccount* account = accounts_get_account(session_get_account_name()); if (account) { @@ -201,7 +195,7 @@ http_file_put(void* userdata) curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, "PUT"); struct curl_slist* headers = NULL; - content_type_header = g_strdup_printf("Content-Type: %s", upload->mime_type); + auto_gchar gchar* content_type_header = g_strdup_printf("Content-Type: %s", upload->mime_type); if (!content_type_header) { content_type_header = g_strdup(FALLBACK_CONTENTTYPE_HEADER); } @@ -209,6 +203,7 @@ http_file_put(void* userdata) headers = curl_slist_append(headers, "Expect:"); // Optional headers + auto_gchar gchar* auth_header = NULL; if (upload->authorization) { auth_header = g_strdup_printf("Authorization: %s", upload->authorization); if (!auth_header) { @@ -216,6 +211,7 @@ http_file_put(void* userdata) } headers = curl_slist_append(headers, auth_header); } + auto_gchar gchar* cookie_header = NULL; if (upload->cookie) { cookie_header = g_strdup_printf("Cookie: %s", upload->cookie); if (!cookie_header) { @@ -223,6 +219,7 @@ http_file_put(void* userdata) } headers = curl_slist_append(headers, cookie_header); } + auto_gchar gchar* expires_header = NULL; if (upload->expires) { expires_header = g_strdup_printf("Expires: %s", upload->expires); if (!expires_header) { @@ -297,13 +294,8 @@ http_file_put(void* userdata) fclose(fh); } free(output.buffer); - g_free(content_type_header); - g_free(auth_header); - g_free(cookie_header); - g_free(expires_header); pthread_mutex_lock(&lock); - g_free(cafile); if (err) { gchar* msg; diff --git a/src/ui/console.c b/src/ui/console.c index 6a212f420..3816831f5 100644 --- a/src/ui/console.c +++ b/src/ui/console.c @@ -160,12 +160,11 @@ cons_bad_cmd_usage(const char* const cmd) void cons_show_error(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_ERROR, "-", "%s", fmt_msg->str); + win_println(wins_get_console(), THEME_ERROR, "-", "%s", fmt_msg->str); g_string_free(fmt_msg, TRUE); va_end(arg); diff --git a/src/ui/core.c b/src/ui/core.c index b5a2eb1ab..9bd872a04 100644 --- a/src/ui/core.c +++ b/src/ui/core.c @@ -93,7 +93,6 @@ static void _ui_close(void) { g_timer_destroy(ui_idle_time); - endwin(); notifier_uninit(); cons_clear_alerts(); wins_destroy(); @@ -102,6 +101,7 @@ _ui_close(void) free_title_bar(); delwin(main_scr); delscreen(set_term(NULL)); + endwin(); } void diff --git a/src/xmpp/chat_session.c b/src/xmpp/chat_session.c index 9834e3317..1c62c3740 100644 --- a/src/xmpp/chat_session.c +++ b/src/xmpp/chat_session.c @@ -69,8 +69,8 @@ static void _chat_session_free(ChatSession* session) { if (session) { - free(session->barejid); free(session->resource); + free(session->barejid); free(session); } } diff --git a/src/xmpp/iq.c b/src/xmpp/iq.c index b8cc3aa58..efbd5f903 100644 --- a/src/xmpp/iq.c +++ b/src/xmpp/iq.c @@ -2571,7 +2571,8 @@ iq_send_stanza(xmpp_stanza_t* const stanza) if (plugin_text) { xmpp_send_raw_string(conn, "%s", plugin_text); } else { - xmpp_send_raw_string(conn, "%s", text); + /* libstrophe does the strlen for us, so simply always pass SIZE_MAX */ + xmpp_send_raw(conn, text, SIZE_MAX); } xmpp_free(connection_get_ctx(), text); } diff --git a/tests/unittests/test_common.c b/tests/unittests/test_common.c index b10f30bbc..98eb712af 100644 --- a/tests/unittests/test_common.c +++ b/tests/unittests/test_common.c @@ -546,7 +546,7 @@ prof_occurrences_of_large_message_tests(void** state) char* haystack = malloc(haystack_sz); const char needle[] = "needle "; while (haystack_sz - haystack_cur > sizeof(needle)) { - memcpy(&haystack[haystack_cur], needle, sizeof(needle) - 1); + memcpy(&haystack[haystack_cur], needle, sizeof(needle)); expected = g_slist_append(expected, GINT_TO_POINTER(haystack_cur)); haystack_cur += sizeof(needle) - 1; } From c0da36c48d9878c7e90a796d97f5670fd686d7aa Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 7 Mar 2025 19:03:10 +0100 Subject: [PATCH 06/13] Rage-cleanup. While trying to get the unit tests working again I stumbled over all those things that I thought could be better^TM. Now we also know "TODO: why does this make the test fail?" - because the unit tests are brittle AF ... and we have to init the subsystems we use in the test, otherwise the cleanup will fail... BTW. you can now also only run a single test ... or a pattern or so ... you'd have to read how `cmocka_set_test_filter()` works exactly. Signed-off-by: Steffen Jaeckel --- Makefile.am | 3 ++ src/command/cmd_ac.c | 1 + src/command/cmd_defs.c | 2 + src/log.c | 58 +++++++++++++++++++--------- src/plugins/api.c | 10 +---- src/plugins/callbacks.c | 8 +++- src/plugins/python_plugins.c | 13 ++----- src/profanity.c | 5 ++- src/tools/autocomplete.c | 7 +--- src/ui/core.c | 3 +- src/ui/inputwin.c | 3 +- src/ui/statusbar.c | 4 +- src/ui/titlebar.c | 1 + src/ui/window_list.c | 3 ++ src/xmpp/avatar.c | 22 +++++++---- src/xmpp/chat_session.c | 8 ++-- src/xmpp/iq.c | 2 - src/xmpp/message.c | 18 ++++++--- tests/unittests/helpers.c | 10 +++++ tests/unittests/test_callbacks.c | 11 ++---- tests/unittests/test_cmd_alias.c | 1 - tests/unittests/test_cmd_bookmark.c | 10 ----- tests/unittests/test_cmd_join.c | 8 ---- tests/unittests/test_muc.c | 1 - tests/unittests/test_server_events.c | 9 +---- tests/unittests/unittests.c | 16 ++++++-- 26 files changed, 133 insertions(+), 104 deletions(-) diff --git a/Makefile.am b/Makefile.am index 5d5e38fdf..99370e820 100644 --- a/Makefile.am +++ b/Makefile.am @@ -357,6 +357,9 @@ check-unit: tests/unittests/unittests format: $(all_c_sources) clang-format -i $(all_c_sources) +format-sources: $(core_sources) $(main_source) + clang-format -i $^ + spell: codespell diff --git a/src/command/cmd_ac.c b/src/command/cmd_ac.c index 3a4a46059..fbb6bb5af 100644 --- a/src/command/cmd_ac.c +++ b/src/command/cmd_ac.c @@ -1648,6 +1648,7 @@ cmd_ac_uninit(void) autocomplete_free(plugins_reload_ac); autocomplete_free(script_show_ac); g_hash_table_destroy(ac_funcs); + ac_funcs = NULL; } static void diff --git a/src/command/cmd_defs.c b/src/command/cmd_defs.c index d432cd0bd..2d773c48c 100644 --- a/src/command/cmd_defs.c +++ b/src/command/cmd_defs.c @@ -2845,7 +2845,9 @@ _cmd_uninit(void) { cmd_ac_uninit(); g_hash_table_destroy(commands); + commands = NULL; g_hash_table_destroy(search_index); + search_index = NULL; } /* diff --git a/src/log.c b/src/log.c index cacf573bc..221db0b8f 100644 --- a/src/log.c +++ b/src/log.c @@ -54,6 +54,8 @@ #define PROF "prof" +static void _log_msg(log_level_t level, const char* const area, const char* const msg); + static FILE* logp; static gchar* mainlogfile = NULL; static gboolean user_provided_log = FALSE; @@ -121,14 +123,22 @@ _log_abbreviation_string_from_level(log_level_t level) } } +static gboolean +_should_log(log_level_t level) +{ + return level >= level_filter && logp; +} + void log_debug(const char* const msg, ...) { + if (!_should_log(PROF_LEVEL_DEBUG)) + return; va_list arg; va_start(arg, msg); GString* fmt_msg = g_string_new(NULL); g_string_vprintf(fmt_msg, msg, arg); - log_msg(PROF_LEVEL_DEBUG, PROF, fmt_msg->str); + _log_msg(PROF_LEVEL_DEBUG, PROF, fmt_msg->str); g_string_free(fmt_msg, TRUE); va_end(arg); } @@ -136,11 +146,13 @@ log_debug(const char* const msg, ...) void log_info(const char* const msg, ...) { + if (!_should_log(PROF_LEVEL_INFO)) + return; va_list arg; va_start(arg, msg); GString* fmt_msg = g_string_new(NULL); g_string_vprintf(fmt_msg, msg, arg); - log_msg(PROF_LEVEL_INFO, PROF, fmt_msg->str); + _log_msg(PROF_LEVEL_INFO, PROF, fmt_msg->str); g_string_free(fmt_msg, TRUE); va_end(arg); } @@ -148,11 +160,13 @@ log_info(const char* const msg, ...) void log_warning(const char* const msg, ...) { + if (!_should_log(PROF_LEVEL_WARN)) + return; va_list arg; va_start(arg, msg); GString* fmt_msg = g_string_new(NULL); g_string_vprintf(fmt_msg, msg, arg); - log_msg(PROF_LEVEL_WARN, PROF, fmt_msg->str); + _log_msg(PROF_LEVEL_WARN, PROF, fmt_msg->str); g_string_free(fmt_msg, TRUE); va_end(arg); } @@ -160,11 +174,13 @@ log_warning(const char* const msg, ...) void log_error(const char* const msg, ...) { + if (!_should_log(PROF_LEVEL_ERROR)) + return; va_list arg; va_start(arg, msg); GString* fmt_msg = g_string_new(NULL); g_string_vprintf(fmt_msg, msg, arg); - log_msg(PROF_LEVEL_ERROR, PROF, fmt_msg->str); + _log_msg(PROF_LEVEL_ERROR, PROF, fmt_msg->str); g_string_free(fmt_msg, TRUE); va_end(arg); } @@ -206,30 +222,36 @@ log_close(void) } } -void -log_msg(log_level_t level, const char* const area, const char* const msg) +static void +_log_msg(log_level_t level, const char* const area, const char* const msg) { - if (level >= level_filter && logp) { - GDateTime* dt = g_date_time_new_now_local(); + GDateTime* dt = g_date_time_new_now_local(); - char* level_str = _log_abbreviation_string_from_level(level); + char* level_str = _log_abbreviation_string_from_level(level); - auto_gchar gchar* date_fmt = g_date_time_format_iso8601(dt); + auto_gchar gchar* date_fmt = g_date_time_format_iso8601(dt); - fprintf(logp, "%s: %s: %s: %s\n", date_fmt, area, level_str, msg); - g_date_time_unref(dt); + fprintf(logp, "%s: %s: %s: %s\n", date_fmt, area, level_str, msg); + g_date_time_unref(dt); - fflush(logp); + fflush(logp); - if (prefs_get_boolean(PREF_LOG_ROTATE) && !user_provided_log) { - long result = ftell(logp); - if (result != -1 && result >= prefs_get_max_log_size()) { - _rotate_log_file(); - } + if (prefs_get_boolean(PREF_LOG_ROTATE) && !user_provided_log) { + long result = ftell(logp); + if (result != -1 && result >= prefs_get_max_log_size()) { + _rotate_log_file(); } } } +void +log_msg(log_level_t level, const char* const area, const char* const msg) +{ + if (!_should_log(level)) + return; + _log_msg(level, area, msg); +} + int log_level_from_string(char* log_level, log_level_t* level) { diff --git a/src/plugins/api.c b/src/plugins/api.c index 91031a16c..97ad72c34 100644 --- a/src/plugins/api.c +++ b/src/plugins/api.c @@ -111,7 +111,7 @@ api_register_command(const char* const plugin_name, const char* command_name, in char** synopsis, const char* description, char* arguments[][2], char** examples, void* callback, void (*callback_exec)(PluginCommand* command, gchar** args), void (*callback_destroy)(void* callback)) { - PluginCommand* command = malloc(sizeof(PluginCommand)); + PluginCommand* command = calloc(1, sizeof(PluginCommand)); command->command_name = strdup(command_name); command->min_args = min_args; command->max_args = max_args; @@ -119,28 +119,22 @@ api_register_command(const char* const plugin_name, const char* command_name, in command->callback_exec = callback_exec; command->callback_destroy = callback_destroy; - CommandHelp* help = malloc(sizeof(CommandHelp)); - - help->tags[0] = NULL; + CommandHelp* help = calloc(1, sizeof(CommandHelp)); int i; for (i = 0; synopsis[i] != NULL; i++) { help->synopsis[i] = strdup(synopsis[i]); } - help->synopsis[i] = NULL; help->desc = strdup(description); for (i = 0; arguments[i][0] != NULL; i++) { help->args[i][0] = strdup(arguments[i][0]); help->args[i][1] = strdup(arguments[i][1]); } - help->args[i][0] = NULL; - help->args[i][1] = NULL; for (i = 0; examples[i] != NULL; i++) { help->examples[i] = strdup(examples[i]); } - help->examples[i] = NULL; command->help = help; diff --git a/src/plugins/callbacks.c b/src/plugins/callbacks.c index 366070663..6852a0ace 100644 --- a/src/plugins/callbacks.c +++ b/src/plugins/callbacks.c @@ -104,7 +104,8 @@ _free_command(PluginCommand* command) } free(command->command_name); - _free_command_help(command->help); + if (command->help) + _free_command_help(command->help); free(command); } @@ -118,6 +119,8 @@ _free_command_hash(GHashTable* command_hash) static void _free_timed_function(PluginTimedFunction* timed_function) { + if (!timed_function) + return; if (timed_function->callback_destroy) { timed_function->callback_destroy(timed_function->callback); } @@ -178,8 +181,11 @@ void callbacks_close(void) { g_hash_table_destroy(p_window_callbacks); + p_window_callbacks = NULL; g_hash_table_destroy(p_timed_functions); + p_timed_functions = NULL; g_hash_table_destroy(p_commands); + p_commands = NULL; } void diff --git a/src/plugins/python_plugins.c b/src/plugins/python_plugins.c index c25bb541e..8cd721dfc 100644 --- a/src/plugins/python_plugins.c +++ b/src/plugins/python_plugins.c @@ -69,12 +69,6 @@ disable_python_threads() PyEval_RestoreThread(thread_state); } -static void -_unref_module(PyObject* module) -{ - Py_XDECREF(module); -} - const char* python_get_version_string(void) { @@ -95,14 +89,15 @@ python_get_version_number(void) void python_env_init(void) { - loaded_modules = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)_unref_module); + loaded_modules = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)Py_XDECREF); python_init_prof(); auto_gchar gchar* plugins_dir = files_get_data_path(DIR_PLUGINS); auto_gchar gchar* path = g_strdup_printf( - "import sys\n" - "sys.path.append(\"%s/\")\n", plugins_dir); + "import sys\n" + "sys.path.append(\"%s/\")\n", + plugins_dir); PyRun_SimpleString(path); python_check_error(); diff --git a/src/profanity.c b/src/profanity.c index 0b639bb16..24e0b0e6b 100644 --- a/src/profanity.c +++ b/src/profanity.c @@ -298,7 +298,10 @@ _call_and_free_shutdown_routine(struct shutdown_routine* r) void prof_shutdown(void) { - g_list_free_full(shutdown_routines, (GDestroyNotify)_call_and_free_shutdown_routine); + if (shutdown_routines) { + g_list_free_full(shutdown_routines, (GDestroyNotify)_call_and_free_shutdown_routine); + shutdown_routines = NULL; + } } static void diff --git a/src/tools/autocomplete.c b/src/tools/autocomplete.c index ebbce5e3a..b7fb00b30 100644 --- a/src/tools/autocomplete.c +++ b/src/tools/autocomplete.c @@ -62,12 +62,7 @@ static gchar* _search(Autocomplete ac, GList* curr, gboolean quote, search_direc Autocomplete autocomplete_new(void) { - Autocomplete new = malloc(sizeof(struct autocomplete_t)); - new->items = NULL; - new->last_found = NULL; - new->search_str = NULL; - - return new; + return calloc(1, sizeof(struct autocomplete_t)); } void diff --git a/src/ui/core.c b/src/ui/core.c index 9bd872a04..91249d416 100644 --- a/src/ui/core.c +++ b/src/ui/core.c @@ -99,9 +99,10 @@ _ui_close(void) inp_close(); status_bar_close(); free_title_bar(); + endwin(); delwin(main_scr); + main_scr = NULL; delscreen(set_term(NULL)); - endwin(); } void diff --git a/src/ui/inputwin.c b/src/ui/inputwin.c index a7e3a1386..0d2113f01 100644 --- a/src/ui/inputwin.c +++ b/src/ui/inputwin.c @@ -164,7 +164,6 @@ create_input_window(void) inp_win = newpad(1, INP_WIN_MAX); wbkgd(inp_win, theme_attrs(THEME_INPUT_TEXT)); - ; keypad(inp_win, TRUE); wmove(inp_win, 0, 0); @@ -274,7 +273,9 @@ inp_close(void) { rl_callback_handler_remove(); delwin(inp_win); + inp_win = NULL; fclose(discard); + discard = NULL; } char* diff --git a/src/ui/statusbar.c b/src/ui/statusbar.c index d60a32e74..917a8ba82 100644 --- a/src/ui/statusbar.c +++ b/src/ui/statusbar.c @@ -120,6 +120,7 @@ void status_bar_close(void) { delwin(statusbar_win); + statusbar_win = NULL; if (statusbar) { if (statusbar->time) { g_free(statusbar->time); @@ -133,10 +134,11 @@ status_bar_close(void) if (statusbar->tabs) { g_hash_table_destroy(statusbar->tabs); } - free(statusbar); + FREE_SET_NULL(statusbar); } if (tz) { g_time_zone_unref(tz); + tz = NULL; } } diff --git a/src/ui/titlebar.c b/src/ui/titlebar.c index 55aacbebb..7ecdc7ff4 100644 --- a/src/ui/titlebar.c +++ b/src/ui/titlebar.c @@ -89,6 +89,7 @@ void free_title_bar(void) { delwin(win); + win = NULL; } void diff --git a/src/ui/window_list.c b/src/ui/window_list.c index fbb9273f1..6838fbd21 100644 --- a/src/ui/window_list.c +++ b/src/ui/window_list.c @@ -1226,8 +1226,11 @@ void wins_destroy(void) { g_hash_table_destroy(windows); + windows = NULL; autocomplete_free(wins_ac); + wins_ac = NULL; autocomplete_free(wins_close_ac); + wins_close_ac = NULL; } ProfWin* diff --git a/src/xmpp/avatar.c b/src/xmpp/avatar.c index 721cd6232..7014698c0 100644 --- a/src/xmpp/avatar.c +++ b/src/xmpp/avatar.c @@ -77,22 +77,30 @@ _free_avatar_data(avatar_metadata* data) } } +static void +_avatar_cleanup(void) +{ + if (looking_for) { + g_hash_table_destroy(looking_for); + } + if (shall_open) { + g_hash_table_destroy(shall_open); + } + looking_for = NULL; + shall_open = NULL; +} + void avatar_pep_subscribe(void) { + prof_add_shutdown_routine(_avatar_cleanup); message_pubsub_event_handler_add(STANZA_NS_USER_AVATAR_METADATA, _avatar_metadata_handler, NULL, NULL); message_pubsub_event_handler_add(STANZA_NS_USER_AVATAR_DATA, _avatar_metadata_handler, NULL, NULL); // caps_add_feature(XMPP_FEATURE_USER_AVATAR_METADATA_NOTIFY); - if (looking_for) { - g_hash_table_destroy(looking_for); - } + _avatar_cleanup(); looking_for = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL); - - if (shall_open) { - g_hash_table_destroy(shall_open); - } shall_open = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL); } diff --git a/src/xmpp/chat_session.c b/src/xmpp/chat_session.c index 1c62c3740..65b6b32f4 100644 --- a/src/xmpp/chat_session.c +++ b/src/xmpp/chat_session.c @@ -56,7 +56,7 @@ _chat_session_new(const char* const barejid, const char* const resource, gboolea assert(barejid != NULL); assert(resource != NULL); - ChatSession* new_session = malloc(sizeof(struct chat_session_t)); + ChatSession* new_session = calloc(1, sizeof(*new_session)); new_session->barejid = strdup(barejid); new_session->resource = strdup(resource); new_session->resource_override = resource_override; @@ -88,8 +88,10 @@ chat_sessions_init(void) void chat_sessions_clear(void) { - if (sessions) + if (sessions) { g_hash_table_remove_all(sessions); + sessions = NULL; + } } void @@ -101,7 +103,7 @@ chat_session_resource_override(const char* const barejid, const char* const reso ChatSession* chat_session_get(const char* const barejid) { - return g_hash_table_lookup(sessions, barejid); + return sessions ? g_hash_table_lookup(sessions, barejid) : NULL; } char* diff --git a/src/xmpp/iq.c b/src/xmpp/iq.c index efbd5f903..c9f66d395 100644 --- a/src/xmpp/iq.c +++ b/src/xmpp/iq.c @@ -320,7 +320,6 @@ void iq_handlers_clear(void) { if (id_handlers) { - g_hash_table_remove_all(id_handlers); g_hash_table_destroy(id_handlers); id_handlers = NULL; } @@ -410,7 +409,6 @@ void iq_rooms_cache_clear(void) { if (rooms_cache) { - g_hash_table_remove_all(rooms_cache); g_hash_table_destroy(rooms_cache); rooms_cache = NULL; } diff --git a/src/xmpp/message.c b/src/xmpp/message.c index 3bf1dc0dc..5c0477b8c 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -320,13 +320,9 @@ _handle_form(xmpp_stanza_t* const stanza) return TRUE; } -void -message_handlers_init(void) +static void +_message_handlers_cleanup(void) { - xmpp_conn_t* const conn = connection_get_conn(); - xmpp_ctx_t* const ctx = connection_get_ctx(); - xmpp_handler_add(conn, _message_handler, NULL, STANZA_NAME_MESSAGE, NULL, ctx); - if (pubsub_event_handlers) { GList* keys = g_hash_table_get_keys(pubsub_event_handlers); GList* curr = keys; @@ -340,7 +336,17 @@ message_handlers_init(void) g_list_free(keys); g_hash_table_destroy(pubsub_event_handlers); } + pubsub_event_handlers = NULL; +} +void +message_handlers_init(void) +{ + prof_add_shutdown_routine(_message_handlers_cleanup); + xmpp_conn_t* const conn = connection_get_conn(); + xmpp_ctx_t* const ctx = connection_get_ctx(); + xmpp_handler_add(conn, _message_handler, NULL, STANZA_NAME_MESSAGE, NULL, ctx); + _message_handlers_cleanup(); pubsub_event_handlers = g_hash_table_new_full(g_str_hash, g_str_equal, free, free); } diff --git a/tests/unittests/helpers.c b/tests/unittests/helpers.c index 3205e460b..f567bcd5c 100644 --- a/tests/unittests/helpers.c +++ b/tests/unittests/helpers.c @@ -11,6 +11,9 @@ #include "helpers.h" #include "config/preferences.h" #include "xmpp/chat_session.h" +#include "command/cmd_defs.h" + +void prof_shutdown(void); void create_config_dir(void** state) @@ -51,18 +54,24 @@ load_preferences(void** state) FILE* f = fopen("./tests/files/xdg_config_home/profanity/profrc", "ab+"); if (f) { prefs_load(NULL); + } else { + return 1; } fclose(f); + cmd_init(); + chat_sessions_init(); return 0; } int close_preferences(void** state) { + chat_sessions_clear(); prefs_close(); remove("./tests/files/xdg_config_home/profanity/profrc"); remove_config_dir(state); rmdir("./tests/files"); + prof_shutdown(); return 0; } @@ -79,6 +88,7 @@ close_chat_sessions(void** state) { chat_sessions_clear(); close_preferences(NULL); + prof_shutdown(); return 0; } diff --git a/tests/unittests/test_callbacks.c b/tests/unittests/test_callbacks.c index a9b1138b2..e545fce13 100644 --- a/tests/unittests/test_callbacks.c +++ b/tests/unittests/test_callbacks.c @@ -16,9 +16,6 @@ returns_no_commands(void** state) GList* commands = plugins_get_command_names(); assert_true(commands == NULL); - - callbacks_close(); - g_list_free(commands); } void @@ -26,15 +23,15 @@ returns_commands(void** state) { callbacks_init(); - PluginCommand* command1 = malloc(sizeof(PluginCommand)); + PluginCommand* command1 = calloc(1, sizeof(PluginCommand)); command1->command_name = strdup("command1"); callbacks_add_command("plugin1", command1); - PluginCommand* command2 = malloc(sizeof(PluginCommand)); + PluginCommand* command2 = calloc(1, sizeof(PluginCommand)); command2->command_name = strdup("command2"); callbacks_add_command("plugin1", command2); - PluginCommand* command3 = malloc(sizeof(PluginCommand)); + PluginCommand* command3 = calloc(1, sizeof(PluginCommand)); command3->command_name = strdup("command3"); callbacks_add_command("plugin2", command3); @@ -61,6 +58,4 @@ returns_commands(void** state) assert_true(foundCommand1 && foundCommand2 && foundCommand3); g_list_free(names); - // TODO: why does this make the test fail? - // callbacks_close(); } diff --git a/tests/unittests/test_cmd_alias.c b/tests/unittests/test_cmd_alias.c index 6d01ee146..d23816bdc 100644 --- a/tests/unittests/test_cmd_alias.c +++ b/tests/unittests/test_cmd_alias.c @@ -84,7 +84,6 @@ cmd_alias_add_shows_message_when_exists(void** state) { gchar* args[] = { "add", "hc", "/help commands", NULL }; - cmd_init(); prefs_add_alias("hc", "/help commands"); cmd_ac_add("/hc"); diff --git a/tests/unittests/test_cmd_bookmark.c b/tests/unittests/test_cmd_bookmark.c index 58201ca66..2859f143c 100644 --- a/tests/unittests/test_cmd_bookmark.c +++ b/tests/unittests/test_cmd_bookmark.c @@ -216,8 +216,6 @@ cmd_bookmark_uses_roomjid_in_room(void** state) gboolean result = cmd_bookmark(&muc_win.window, CMD_BOOKMARK, args); assert_true(result); - - muc_close(); } void @@ -243,8 +241,6 @@ cmd_bookmark_add_uses_roomjid_in_room(void** state) gboolean result = cmd_bookmark(&muc_win.window, CMD_BOOKMARK, args); assert_true(result); - - muc_close(); } void @@ -271,8 +267,6 @@ cmd_bookmark_add_uses_supplied_jid_in_room(void** state) gboolean result = cmd_bookmark(&muc_win.window, CMD_BOOKMARK, args); assert_true(result); - - muc_close(); } void @@ -401,8 +395,6 @@ cmd_bookmark_remove_uses_roomjid_in_room(void** state) gboolean result = cmd_bookmark(&muc_win.window, CMD_BOOKMARK, args); assert_true(result); - - muc_close(); } void @@ -426,6 +418,4 @@ cmd_bookmark_remove_uses_supplied_jid_in_room(void** state) gboolean result = cmd_bookmark(&muc_win.window, CMD_BOOKMARK, args); assert_true(result); - - muc_close(); } diff --git a/tests/unittests/test_cmd_join.c b/tests/unittests/test_cmd_join.c index 6a77dd4d6..8e1ef4a98 100644 --- a/tests/unittests/test_cmd_join.c +++ b/tests/unittests/test_cmd_join.c @@ -87,8 +87,6 @@ cmd_join_uses_account_mucservice_when_no_service_specified(void** state) gboolean result = cmd_join(NULL, CMD_JOIN, args); assert_true(result); - - muc_close(); } void @@ -115,8 +113,6 @@ cmd_join_uses_supplied_nick(void** state) gboolean result = cmd_join(NULL, CMD_JOIN, args); assert_true(result); - - muc_close(); } void @@ -143,8 +139,6 @@ cmd_join_uses_account_nick_when_not_supplied(void** state) gboolean result = cmd_join(NULL, CMD_JOIN, args); assert_true(result); - - muc_close(); } void @@ -174,6 +168,4 @@ cmd_join_uses_password_when_supplied(void** state) gboolean result = cmd_join(NULL, CMD_JOIN, args); assert_true(result); - - muc_close(); } diff --git a/tests/unittests/test_muc.c b/tests/unittests/test_muc.c index 977e2de86..71fecd5d6 100644 --- a/tests/unittests/test_muc.c +++ b/tests/unittests/test_muc.c @@ -16,7 +16,6 @@ muc_before_test(void** state) int muc_after_test(void** state) { - muc_close(); return 0; } diff --git a/tests/unittests/test_server_events.c b/tests/unittests/test_server_events.c index feed7709d..fbed077a8 100644 --- a/tests/unittests/test_server_events.c +++ b/tests/unittests/test_server_events.c @@ -17,9 +17,6 @@ #include "plugins/plugins.h" #include "ui/window_list.h" - -void prof_shutdown(void); - void console_shows_online_presence_when_set_online(void** state) { @@ -38,7 +35,6 @@ console_shows_online_presence_when_set_online(void** state) sv_ev_contact_online(barejid, resource, NULL, NULL); roster_destroy(); - prof_shutdown(); } void @@ -59,7 +55,6 @@ console_shows_online_presence_when_set_all(void** state) sv_ev_contact_online(barejid, resource, NULL, NULL); roster_destroy(); - prof_shutdown(); } void @@ -80,7 +75,6 @@ console_shows_dnd_presence_when_set_all(void** state) sv_ev_contact_online(barejid, resource, NULL, NULL); roster_destroy(); - prof_shutdown(); } void @@ -96,7 +90,7 @@ handle_offline_removes_chat_session(void** state) Resource* resourcep = resource_new(resource, RESOURCE_ONLINE, NULL, 10); roster_update_presence(barejid, resourcep, NULL); chat_session_recipient_active(barejid, resource, FALSE); - ProfConsoleWin* console = malloc(sizeof(ProfConsoleWin)); + ProfConsoleWin* console = calloc(1, sizeof(ProfConsoleWin)); will_return(win_create_console, &console->window); wins_init(); sv_ev_contact_offline(barejid, resource, NULL); @@ -106,7 +100,6 @@ handle_offline_removes_chat_session(void** state) roster_destroy(); chat_sessions_clear(); - prof_shutdown(); } void diff --git a/tests/unittests/unittests.c b/tests/unittests/unittests.c index c28f3f2ec..8e6c1b9b5 100644 --- a/tests/unittests/unittests.c +++ b/tests/unittests/unittests.c @@ -57,6 +57,9 @@ main(int argc, char* argv[]) printf(" MB_CUR_MAX: %d\n", (int)MB_CUR_MAX); printf(" MB_LEN_MAX: %d\n", (int)MB_LEN_MAX); + if (argc > 1) + cmocka_set_test_filter(argv[1]); + const struct CMUnitTest all_tests[] = { cmocka_unit_test(replace_one_substr), @@ -460,7 +463,9 @@ main(int argc, char* argv[]) cmocka_unit_test_setup_teardown(handle_offline_removes_chat_session, load_preferences, close_preferences), - cmocka_unit_test(lost_connection_clears_chat_sessions), + cmocka_unit_test_setup_teardown(lost_connection_clears_chat_sessions, + load_preferences, + close_preferences), cmocka_unit_test(cmd_alias_add_shows_usage_when_no_args), cmocka_unit_test(cmd_alias_add_shows_usage_when_no_value), @@ -623,8 +628,12 @@ main(int argc, char* argv[]) cmocka_unit_test(prof_whole_occurrences_tests), cmocka_unit_test(prof_occurrences_of_large_message_tests), - cmocka_unit_test(returns_no_commands), - cmocka_unit_test(returns_commands), + cmocka_unit_test_setup_teardown(returns_no_commands, + load_preferences, + close_preferences), + cmocka_unit_test_setup_teardown(returns_commands, + load_preferences, + close_preferences), cmocka_unit_test(returns_empty_list_when_none), cmocka_unit_test(returns_added_feature), @@ -634,6 +643,5 @@ main(int argc, char* argv[]) cmocka_unit_test(removes_plugin_features), cmocka_unit_test(does_not_remove_feature_when_more_than_one_reference), }; - return cmocka_run_group_tests(all_tests, NULL, NULL); } From 9fc0326428eae41f15b60e27b77b4129b1153c61 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 7 Mar 2025 20:38:18 +0100 Subject: [PATCH 07/13] Add Valgrind to CI * Also pass `$*` to `configure` when invoking `ci-build.sh`, so one can e.g. run `./ci-build.sh --without-xscreensaver` Signed-off-by: Steffen Jaeckel --- Dockerfile.arch | 1 + Dockerfile.debian | 1 + Dockerfile.fedora | 1 + Dockerfile.tumbleweed | 1 + Dockerfile.ubuntu | 1 + Makefile.am | 3 + ci-build.sh | 24 ++- configure.ac | 6 + m4/ax_valgrind_check.m4 | 239 ++++++++++++++++++++++++++ prof.supp | 21 +++ src/xmpp/chat_session.c | 2 +- src/xmpp/muc.c | 10 +- tests/unittests/test_callbacks.c | 5 +- tests/unittests/test_cmd_bookmark.c | 10 -- tests/unittests/test_cmd_disconnect.c | 2 +- tests/unittests/test_cmd_join.c | 8 - tests/unittests/test_common.c | 2 +- tests/unittests/test_muc.c | 3 + tests/unittests/unittests.c | 20 ++- 19 files changed, 319 insertions(+), 41 deletions(-) create mode 100644 m4/ax_valgrind_check.m4 diff --git a/Dockerfile.arch b/Dockerfile.arch index 3fe70dff8..fa2afb871 100644 --- a/Dockerfile.arch +++ b/Dockerfile.arch @@ -28,6 +28,7 @@ RUN pacman -Syu --noconfirm && pacman -S --needed --noconfirm \ python \ wget \ sqlite \ + valgrind \ gdk-pixbuf2 \ qrencode diff --git a/Dockerfile.debian b/Dockerfile.debian index 6da1f414f..114aec28a 100644 --- a/Dockerfile.debian +++ b/Dockerfile.debian @@ -27,6 +27,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ pkg-config \ python3-dev \ python-dev-is-python3 \ + valgrind \ libsqlite3-dev \ libgdk-pixbuf-2.0-dev \ libqrencode-dev diff --git a/Dockerfile.fedora b/Dockerfile.fedora index effefc5c5..56499b30c 100644 --- a/Dockerfile.fedora +++ b/Dockerfile.fedora @@ -33,6 +33,7 @@ RUN dnf install -y \ readline-devel \ openssl-devel \ sqlite-devel \ + valgrind \ gdk-pixbuf2-devel \ qrencode-devel diff --git a/Dockerfile.tumbleweed b/Dockerfile.tumbleweed index 5cd6c20ce..1777d1708 100644 --- a/Dockerfile.tumbleweed +++ b/Dockerfile.tumbleweed @@ -33,6 +33,7 @@ RUN zypper --non-interactive in --no-recommends \ python310-devel \ readline-devel \ sqlite3-devel \ + valgrind \ gdk-pixbuf-devel \ qrencode-devel diff --git a/Dockerfile.ubuntu b/Dockerfile.ubuntu index 853544c0f..7f100dddb 100644 --- a/Dockerfile.ubuntu +++ b/Dockerfile.ubuntu @@ -28,6 +28,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ pkg-config \ python3-dev \ python-dev-is-python3 \ + valgrind \ libsqlite3-dev \ libgdk-pixbuf-2.0-dev \ libqrencode-dev diff --git a/Makefile.am b/Makefile.am index 99370e820..e9a1c365a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -354,6 +354,9 @@ my-prof.supp: check-unit: tests/unittests/unittests tests/unittests/unittests +@VALGRIND_CHECK_RULES@ +VALGRIND_SUPPRESSIONS_FILES=prof.supp + format: $(all_c_sources) clang-format -i $(all_c_sources) diff --git a/ci-build.sh b/ci-build.sh index 0e84cae93..963db462e 100755 --- a/ci-build.sh +++ b/ci-build.sh @@ -13,6 +13,7 @@ error_handler() log_content ./config.log log_content ./test-suite.log + log_content ./test-suite-memcheck.log echo echo "Error ${ERR_CODE} with command '${BASH_COMMAND}' on line ${BASH_LINENO[0]}. Exiting." @@ -39,7 +40,9 @@ tests=() MAKE="make --quiet -j$(num_cores)" CC="gcc" -case $(uname | tr '[:upper:]' '[:lower:]') in +ARCH="$(uname | tr '[:upper:]' '[:lower:]')" + +case "$ARCH" in linux*) tests=( "--enable-notifications --enable-icons-and-clipboard --enable-otr --enable-pgp @@ -114,14 +117,29 @@ case $(uname | tr '[:upper:]' '[:lower:]') in ;; esac +case "$ARCH" in + linux*) + echo + echo "--> Building with ./configure ${tests[0]} --enable-valgrind $*" + echo + + # shellcheck disable=SC2086 + ./configure ${tests[0]} --enable-valgrind $* + + $MAKE CC="${CC}" + $MAKE check-valgrind + $MAKE distclean + ;; +esac + for features in "${tests[@]}" do echo - echo "--> Building with ./configure ${features}" + echo "--> Building with ./configure ${features} $*" echo # shellcheck disable=SC2086 - ./configure $features + ./configure $features $* $MAKE CC="${CC}" $MAKE check diff --git a/configure.ac b/configure.ac index d645c1407..844778fee 100644 --- a/configure.ac +++ b/configure.ac @@ -70,6 +70,12 @@ AC_ARG_ENABLE([gdk-pixbuf], AC_ARG_ENABLE([omemo-qrcode], [AS_HELP_STRING([--enable-omemo-qrcode], [enable ability to display omemo qr code])]) +m4_include([m4/ax_valgrind_check.m4]) +AX_VALGRIND_DFLT([drd], [off]) +AX_VALGRIND_DFLT([helgrind], [off]) +AX_VALGRIND_DFLT([sgcheck], [off]) +AX_VALGRIND_CHECK + # Required dependencies AC_CHECK_FUNCS([atexit memset strdup strstr]) diff --git a/m4/ax_valgrind_check.m4 b/m4/ax_valgrind_check.m4 new file mode 100644 index 000000000..a1b58307b --- /dev/null +++ b/m4/ax_valgrind_check.m4 @@ -0,0 +1,239 @@ +# =========================================================================== +# https://www.gnu.org/software/autoconf-archive/ax_valgrind_check.html +# =========================================================================== +# +# SYNOPSIS +# +# AX_VALGRIND_DFLT(memcheck|helgrind|drd|sgcheck, on|off) +# AX_VALGRIND_CHECK() +# +# DESCRIPTION +# +# AX_VALGRIND_CHECK checks whether Valgrind is present and, if so, allows +# running `make check` under a variety of Valgrind tools to check for +# memory and threading errors. +# +# Defines VALGRIND_CHECK_RULES which should be substituted in your +# Makefile; and $enable_valgrind which can be used in subsequent configure +# output. VALGRIND_ENABLED is defined and substituted, and corresponds to +# the value of the --enable-valgrind option, which defaults to being +# enabled if Valgrind is installed and disabled otherwise. Individual +# Valgrind tools can be disabled via --disable-valgrind-, the +# default is configurable via the AX_VALGRIND_DFLT command or is to use +# all commands not disabled via AX_VALGRIND_DFLT. All AX_VALGRIND_DFLT +# calls must be made before the call to AX_VALGRIND_CHECK. +# +# If unit tests are written using a shell script and automake's +# LOG_COMPILER system, the $(VALGRIND) variable can be used within the +# shell scripts to enable Valgrind, as described here: +# +# https://www.gnu.org/software/gnulib/manual/html_node/Running-self_002dtests-under-valgrind.html +# +# Usage example: +# +# configure.ac: +# +# AX_VALGRIND_DFLT([sgcheck], [off]) +# AX_VALGRIND_CHECK +# +# in each Makefile.am with tests: +# +# @VALGRIND_CHECK_RULES@ +# VALGRIND_SUPPRESSIONS_FILES = my-project.supp +# EXTRA_DIST = my-project.supp +# +# This results in a "check-valgrind" rule being added. Running `make +# check-valgrind` in that directory will recursively run the module's test +# suite (`make check`) once for each of the available Valgrind tools (out +# of memcheck, helgrind and drd) while the sgcheck will be skipped unless +# enabled again on the commandline with --enable-valgrind-sgcheck. The +# results for each check will be output to test-suite-$toolname.log. The +# target will succeed if there are zero errors and fail otherwise. +# +# Alternatively, a "check-valgrind-$TOOL" rule will be added, for $TOOL in +# memcheck, helgrind, drd and sgcheck. These are useful because often only +# some of those tools can be ran cleanly on a codebase. +# +# The macro supports running with and without libtool. +# +# LICENSE +# +# Copyright (c) 2014, 2015, 2016 Philip Withnall +# +# Copying and distribution of this file, with or without modification, are +# permitted in any medium without royalty provided the copyright notice +# and this notice are preserved. This file is offered as-is, without any +# warranty. + +#serial 23 + +dnl Configured tools +m4_define([valgrind_tool_list], [[memcheck], [helgrind], [drd], [sgcheck]]) +m4_set_add_all([valgrind_exp_tool_set], [sgcheck]) +m4_foreach([vgtool], [valgrind_tool_list], + [m4_define([en_dflt_valgrind_]vgtool, [on])]) + +AC_DEFUN([AX_VALGRIND_DFLT],[ + m4_define([en_dflt_valgrind_$1], [$2]) +])dnl + +AC_DEFUN([AX_VALGRIND_CHECK],[ + AM_EXTRA_RECURSIVE_TARGETS([check-valgrind]) + m4_foreach([vgtool], [valgrind_tool_list], + [AM_EXTRA_RECURSIVE_TARGETS([check-valgrind-]vgtool)]) + + dnl Check for --enable-valgrind + AC_ARG_ENABLE([valgrind], + [AS_HELP_STRING([--enable-valgrind], [Whether to enable Valgrind on the unit tests])], + [enable_valgrind=$enableval],[enable_valgrind=]) + + AS_IF([test "$enable_valgrind" != "no"],[ + # Check for Valgrind. + AC_CHECK_PROG([VALGRIND],[valgrind],[valgrind]) + AS_IF([test "$VALGRIND" = ""],[ + AS_IF([test "$enable_valgrind" = "yes"],[ + AC_MSG_ERROR([Could not find valgrind; either install it or reconfigure with --disable-valgrind]) + ],[ + enable_valgrind=no + ]) + ],[ + enable_valgrind=yes + ]) + ]) + + AM_CONDITIONAL([VALGRIND_ENABLED],[test "$enable_valgrind" = "yes"]) + AC_SUBST([VALGRIND_ENABLED],[$enable_valgrind]) + + # Check for Valgrind tools we care about. + [valgrind_enabled_tools=] + m4_foreach([vgtool],[valgrind_tool_list],[ + AC_ARG_ENABLE([valgrind-]vgtool, + m4_if(m4_defn([en_dflt_valgrind_]vgtool),[off],dnl +[AS_HELP_STRING([--enable-valgrind-]vgtool, [Whether to use ]vgtool[ during the Valgrind tests])],dnl +[AS_HELP_STRING([--disable-valgrind-]vgtool, [Whether to skip ]vgtool[ during the Valgrind tests])]), + [enable_valgrind_]vgtool[=$enableval], + [enable_valgrind_]vgtool[=]) + AS_IF([test "$enable_valgrind" = "no"],[ + enable_valgrind_]vgtool[=no], + [test "$enable_valgrind_]vgtool[" ]dnl +m4_if(m4_defn([en_dflt_valgrind_]vgtool), [off], [= "yes"], [!= "no"]),[ + AC_CACHE_CHECK([for Valgrind tool ]vgtool, + [ax_cv_valgrind_tool_]vgtool,[ + ax_cv_valgrind_tool_]vgtool[=no + m4_set_contains([valgrind_exp_tool_set],vgtool, + [m4_define([vgtoolx],[exp-]vgtool)], + [m4_define([vgtoolx],vgtool)]) + AS_IF([`$VALGRIND --tool=]vgtoolx[ --help >/dev/null 2>&1`],[ + ax_cv_valgrind_tool_]vgtool[=yes + ]) + ]) + AS_IF([test "$ax_cv_valgrind_tool_]vgtool[" = "no"],[ + AS_IF([test "$enable_valgrind_]vgtool[" = "yes"],[ + AC_MSG_ERROR([Valgrind does not support ]vgtool[; reconfigure with --disable-valgrind-]vgtool) + ],[ + enable_valgrind_]vgtool[=no + ]) + ],[ + enable_valgrind_]vgtool[=yes + ]) + ]) + AS_IF([test "$enable_valgrind_]vgtool[" = "yes"],[ + valgrind_enabled_tools="$valgrind_enabled_tools ]m4_bpatsubst(vgtool,[^exp-])[" + ]) + AC_SUBST([ENABLE_VALGRIND_]vgtool,[$enable_valgrind_]vgtool) + ]) + AC_SUBST([valgrind_tools],["]m4_join([ ], valgrind_tool_list)["]) + AC_SUBST([valgrind_enabled_tools],[$valgrind_enabled_tools]) + +[VALGRIND_CHECK_RULES=' +# Valgrind check +# +# Optional: +# - VALGRIND_SUPPRESSIONS_FILES: Space-separated list of Valgrind suppressions +# files to load. (Default: empty) +# - VALGRIND_FLAGS: General flags to pass to all Valgrind tools. +# (Default: --num-callers=30) +# - VALGRIND_$toolname_FLAGS: Flags to pass to Valgrind $toolname (one of: +# memcheck, helgrind, drd, sgcheck). (Default: various) + +# Optional variables +VALGRIND_SUPPRESSIONS ?= $(addprefix --suppressions=,$(VALGRIND_SUPPRESSIONS_FILES)) +VALGRIND_FLAGS ?= --num-callers=30 +VALGRIND_memcheck_FLAGS ?= --leak-check=full --show-reachable=no +VALGRIND_helgrind_FLAGS ?= --history-level=approx +VALGRIND_drd_FLAGS ?= +VALGRIND_sgcheck_FLAGS ?= + +# Internal use +valgrind_log_files = $(addprefix test-suite-,$(addsuffix .log,$(valgrind_tools))) + +valgrind_memcheck_flags = --tool=memcheck $(VALGRIND_memcheck_FLAGS) +valgrind_helgrind_flags = --tool=helgrind $(VALGRIND_helgrind_FLAGS) +valgrind_drd_flags = --tool=drd $(VALGRIND_drd_FLAGS) +valgrind_sgcheck_flags = --tool=exp-sgcheck $(VALGRIND_sgcheck_FLAGS) + +valgrind_quiet = $(valgrind_quiet_$(V)) +valgrind_quiet_ = $(valgrind_quiet_$(AM_DEFAULT_VERBOSITY)) +valgrind_quiet_0 = --quiet +valgrind_v_use = $(valgrind_v_use_$(V)) +valgrind_v_use_ = $(valgrind_v_use_$(AM_DEFAULT_VERBOSITY)) +valgrind_v_use_0 = @echo " USE " $(patsubst check-valgrind-%-local,%,$''@):; + +# Support running with and without libtool. +ifneq ($(LIBTOOL),) +valgrind_lt = $(LIBTOOL) $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=execute +else +valgrind_lt = +endif + +# Use recursive makes in order to ignore errors during check +check-valgrind-local: +ifeq ($(VALGRIND_ENABLED),yes) + $(A''M_V_at)$(MAKE) $(AM_MAKEFLAGS) -k \ + $(foreach tool, $(valgrind_enabled_tools), check-valgrind-$(tool)) +else + @echo "Need to reconfigure with --enable-valgrind" +endif + +# Valgrind running +VALGRIND_TESTS_ENVIRONMENT = \ + $(TESTS_ENVIRONMENT) \ + env VALGRIND=$(VALGRIND) \ + G_SLICE=always-malloc,debug-blocks \ + G_DEBUG=fatal-warnings,fatal-criticals,gc-friendly + +VALGRIND_LOG_COMPILER = \ + $(valgrind_lt) \ + $(VALGRIND) $(VALGRIND_SUPPRESSIONS) --error-exitcode=1 $(VALGRIND_FLAGS) + +define valgrind_tool_rule +check-valgrind-$(1)-local: +ifeq ($$(VALGRIND_ENABLED)-$$(ENABLE_VALGRIND_$(1)),yes-yes) +ifneq ($$(TESTS),) + $$(valgrind_v_use)$$(MAKE) check-TESTS \ + TESTS_ENVIRONMENT="$$(VALGRIND_TESTS_ENVIRONMENT)" \ + LOG_COMPILER="$$(VALGRIND_LOG_COMPILER)" \ + LOG_FLAGS="$$(valgrind_$(1)_flags)" \ + TEST_SUITE_LOG=test-suite-$(1).log +endif +else ifeq ($$(VALGRIND_ENABLED),yes) + @echo "Need to reconfigure with --enable-valgrind-$(1)" +else + @echo "Need to reconfigure with --enable-valgrind" +endif +endef + +$(foreach tool,$(valgrind_tools),$(eval $(call valgrind_tool_rule,$(tool)))) + +A''M_DISTCHECK_CONFIGURE_FLAGS ?= +A''M_DISTCHECK_CONFIGURE_FLAGS += --disable-valgrind + +MOSTLYCLEANFILES ?= +MOSTLYCLEANFILES += $(valgrind_log_files) + +.PHONY: check-valgrind $(addprefix check-valgrind-,$(valgrind_tools)) +'] + + AC_SUBST([VALGRIND_CHECK_RULES]) + m4_ifdef([_AM_SUBST_NOTMAKE], [_AM_SUBST_NOTMAKE([VALGRIND_CHECK_RULES])]) +]) \ No newline at end of file diff --git a/prof.supp b/prof.supp index 3d0c58804..36f9ef71d 100644 --- a/prof.supp +++ b/prof.supp @@ -252,6 +252,27 @@ fun:Py_InitializeEx fun:python_env_init } +{ + Custom Inittab can only be cleaned by calling PyMain, which we don't do + Memcheck:Leak + ... + fun:PyImport_ExtendInittab + fun:PyImport_AppendInittab + fun:python_init_prof + ... +} + +# Specific for unit test issues +{ + Libc TLS allocation + Memcheck:Leak + ... + fun:allocate_dtv + fun:_dl_allocate_tls + ... + fun:unique_filename_from_url + ... +} # AUTO-GENERATED START diff --git a/src/xmpp/chat_session.c b/src/xmpp/chat_session.c index 65b6b32f4..0f5f03c2c 100644 --- a/src/xmpp/chat_session.c +++ b/src/xmpp/chat_session.c @@ -89,7 +89,7 @@ void chat_sessions_clear(void) { if (sessions) { - g_hash_table_remove_all(sessions); + g_hash_table_destroy(sessions); sessions = NULL; } } diff --git a/src/xmpp/muc.c b/src/xmpp/muc.c index b6bb57c9d..312ac941e 100644 --- a/src/xmpp/muc.c +++ b/src/xmpp/muc.c @@ -94,14 +94,14 @@ static void _occupant_free(Occupant* occupant); static void _muc_close(void) { - autocomplete_free(invite_ac); - autocomplete_free(confservers_ac); - g_hash_table_destroy(rooms); g_hash_table_destroy(invite_passwords); - rooms = NULL; invite_passwords = NULL; - invite_ac = NULL; + g_hash_table_destroy(rooms); + rooms = NULL; + autocomplete_free(confservers_ac); confservers_ac = NULL; + autocomplete_free(invite_ac); + invite_ac = NULL; } void diff --git a/tests/unittests/test_callbacks.c b/tests/unittests/test_callbacks.c index e545fce13..678c6415f 100644 --- a/tests/unittests/test_callbacks.c +++ b/tests/unittests/test_callbacks.c @@ -12,7 +12,7 @@ void returns_no_commands(void** state) { - callbacks_init(); + plugins_init(); GList* commands = plugins_get_command_names(); assert_true(commands == NULL); @@ -21,8 +21,7 @@ returns_no_commands(void** state) void returns_commands(void** state) { - callbacks_init(); - + plugins_init(); PluginCommand* command1 = calloc(1, sizeof(PluginCommand)); command1->command_name = strdup("command1"); callbacks_add_command("plugin1", command1); diff --git a/tests/unittests/test_cmd_bookmark.c b/tests/unittests/test_cmd_bookmark.c index 2859f143c..c77e0bf16 100644 --- a/tests/unittests/test_cmd_bookmark.c +++ b/tests/unittests/test_cmd_bookmark.c @@ -196,8 +196,6 @@ cmd_bookmark_add_adds_bookmark_with_jid(void** state) void cmd_bookmark_uses_roomjid_in_room(void** state) { - muc_init(); - gchar* args[] = { NULL }; ProfMucWin muc_win; muc_win.window.type = WIN_MUC; @@ -221,8 +219,6 @@ cmd_bookmark_uses_roomjid_in_room(void** state) void cmd_bookmark_add_uses_roomjid_in_room(void** state) { - muc_init(); - gchar* args[] = { "add", NULL }; ProfMucWin muc_win; muc_win.window.type = WIN_MUC; @@ -246,8 +242,6 @@ cmd_bookmark_add_uses_roomjid_in_room(void** state) void cmd_bookmark_add_uses_supplied_jid_in_room(void** state) { - muc_init(); - char* jid = "room1@conf.server"; gchar* args[] = { "add", jid, NULL }; ProfMucWin muc_win; @@ -378,8 +372,6 @@ cmd_bookmark_remove_shows_message_when_no_bookmark(void** state) void cmd_bookmark_remove_uses_roomjid_in_room(void** state) { - muc_init(); - gchar* args[] = { "remove", NULL }; ProfMucWin muc_win; muc_win.window.type = WIN_MUC; @@ -400,8 +392,6 @@ cmd_bookmark_remove_uses_roomjid_in_room(void** state) void cmd_bookmark_remove_uses_supplied_jid_in_room(void** state) { - muc_init(); - char* jid = "room1@conf.server"; gchar* args[] = { "remove", jid, NULL }; ProfMucWin muc_win; diff --git a/tests/unittests/test_cmd_disconnect.c b/tests/unittests/test_cmd_disconnect.c index ac3c36dad..227c7fbc9 100644 --- a/tests/unittests/test_cmd_disconnect.c +++ b/tests/unittests/test_cmd_disconnect.c @@ -23,7 +23,7 @@ clears_chat_sessions(void** state) chat_session_recipient_active("mike@server.org", "work", FALSE); will_return(connection_get_status, JABBER_CONNECTED); - will_return(connection_get_barejid, strdup("myjid@myserver.com")); + will_return(connection_get_barejid, "myjid@myserver.com"); expect_any_cons_show(); gboolean result = cmd_disconnect(NULL, CMD_DISCONNECT, NULL); diff --git a/tests/unittests/test_cmd_join.c b/tests/unittests/test_cmd_join.c index 8e1ef4a98..e933dc894 100644 --- a/tests/unittests/test_cmd_join.c +++ b/tests/unittests/test_cmd_join.c @@ -73,8 +73,6 @@ cmd_join_uses_account_mucservice_when_no_service_specified(void** state) ProfAccount* account = account_new(account_name, g_strdup("user@server.org"), NULL, NULL, TRUE, NULL, 0, g_strdup("laptop"), NULL, NULL, 0, 0, 0, 0, 0, account_service, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0); - muc_init(); - will_return(connection_get_status, JABBER_CONNECTED); will_return(session_get_account_name, account_name); @@ -99,8 +97,6 @@ cmd_join_uses_supplied_nick(void** state) ProfAccount* account = account_new(account_name, g_strdup("user@server.org"), NULL, NULL, TRUE, NULL, 0, g_strdup("laptop"), NULL, NULL, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0); - muc_init(); - will_return(connection_get_status, JABBER_CONNECTED); will_return(session_get_account_name, account_name); @@ -125,8 +121,6 @@ cmd_join_uses_account_nick_when_not_supplied(void** state) ProfAccount* account = account_new(account_name, g_strdup("user@server.org"), NULL, NULL, TRUE, NULL, 0, g_strdup("laptop"), NULL, NULL, 0, 0, 0, 0, 0, NULL, account_nick, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0); - muc_init(); - will_return(connection_get_status, JABBER_CONNECTED); will_return(session_get_account_name, account_name); @@ -154,8 +148,6 @@ cmd_join_uses_password_when_supplied(void** state) ProfAccount* account = account_new(account_name, g_strdup("user@server.org"), NULL, NULL, TRUE, NULL, 0, g_strdup("laptop"), NULL, NULL, 0, 0, 0, 0, 0, account_service, account_nick, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0); - muc_init(); - will_return(connection_get_status, JABBER_CONNECTED); will_return(session_get_account_name, account_name); diff --git a/tests/unittests/test_common.c b/tests/unittests/test_common.c index 98eb712af..f381947b8 100644 --- a/tests/unittests/test_common.c +++ b/tests/unittests/test_common.c @@ -543,7 +543,7 @@ prof_occurrences_of_large_message_tests(void** state) */ const size_t haystack_sz = 1024; size_t haystack_cur = 0; - char* haystack = malloc(haystack_sz); + auto_char char* haystack = malloc(haystack_sz); const char needle[] = "needle "; while (haystack_sz - haystack_cur > sizeof(needle)) { memcpy(&haystack[haystack_cur], needle, sizeof(needle)); diff --git a/tests/unittests/test_muc.c b/tests/unittests/test_muc.c index 71fecd5d6..57fd79e8a 100644 --- a/tests/unittests/test_muc.c +++ b/tests/unittests/test_muc.c @@ -6,6 +6,8 @@ #include "xmpp/muc.h" +void prof_shutdown(void); + int muc_before_test(void** state) { @@ -16,6 +18,7 @@ muc_before_test(void** state) int muc_after_test(void** state) { + prof_shutdown(); return 0; } diff --git a/tests/unittests/unittests.c b/tests/unittests/unittests.c index 8e6c1b9b5..e0d4d792a 100644 --- a/tests/unittests/unittests.c +++ b/tests/unittests/unittests.c @@ -39,6 +39,8 @@ #include "test_callbacks.h" #include "test_plugins_disco.h" +#define muc_unit_test(f) cmocka_unit_test_setup_teardown(f, muc_before_test, muc_after_test) + int main(int argc, char* argv[]) { @@ -501,11 +503,11 @@ main(int argc, char* argv[]) cmocka_unit_test(cmd_bookmark_list_shows_bookmarks), cmocka_unit_test(cmd_bookmark_add_shows_message_when_invalid_jid), cmocka_unit_test(cmd_bookmark_add_adds_bookmark_with_jid), - cmocka_unit_test(cmd_bookmark_uses_roomjid_in_room), - cmocka_unit_test(cmd_bookmark_add_uses_roomjid_in_room), - cmocka_unit_test(cmd_bookmark_add_uses_supplied_jid_in_room), - cmocka_unit_test(cmd_bookmark_remove_uses_roomjid_in_room), - cmocka_unit_test(cmd_bookmark_remove_uses_supplied_jid_in_room), + muc_unit_test(cmd_bookmark_uses_roomjid_in_room), + muc_unit_test(cmd_bookmark_add_uses_roomjid_in_room), + muc_unit_test(cmd_bookmark_add_uses_supplied_jid_in_room), + muc_unit_test(cmd_bookmark_remove_uses_roomjid_in_room), + muc_unit_test(cmd_bookmark_remove_uses_supplied_jid_in_room), cmocka_unit_test(cmd_bookmark_add_adds_bookmark_with_jid_nick), cmocka_unit_test(cmd_bookmark_add_adds_bookmark_with_jid_autojoin), cmocka_unit_test(cmd_bookmark_add_adds_bookmark_with_jid_nick_autojoin), @@ -576,10 +578,10 @@ main(int argc, char* argv[]) cmocka_unit_test(cmd_join_shows_message_when_connecting), cmocka_unit_test(cmd_join_shows_message_when_disconnected), cmocka_unit_test(cmd_join_shows_error_message_when_invalid_room_jid), - cmocka_unit_test(cmd_join_uses_account_mucservice_when_no_service_specified), - cmocka_unit_test(cmd_join_uses_supplied_nick), - cmocka_unit_test(cmd_join_uses_account_nick_when_not_supplied), - cmocka_unit_test(cmd_join_uses_password_when_supplied), + muc_unit_test(cmd_join_uses_account_mucservice_when_no_service_specified), + muc_unit_test(cmd_join_uses_supplied_nick), + muc_unit_test(cmd_join_uses_account_nick_when_not_supplied), + muc_unit_test(cmd_join_uses_password_when_supplied), cmocka_unit_test(cmd_roster_shows_message_when_disconnecting), cmocka_unit_test(cmd_roster_shows_message_when_connecting), From 95c2199ca22c3f0dd7b782dbaf792ba0a12a4e2c Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 7 Mar 2025 22:31:58 +0100 Subject: [PATCH 08/13] Some more memory improvements * Less leaks * Less allocations Signed-off-by: Steffen Jaeckel --- src/event/server_events.c | 2 +- src/otr/otr.c | 4 ++++ src/ui/inputwin.c | 6 +++--- src/ui/notifier.c | 8 +++++++- src/ui/tray.c | 22 +++++++++++++++------- src/xmpp/connection.c | 4 ++++ src/xmpp/iq.c | 29 +++++++++++++++++++++++------ src/xmpp/message.c | 2 +- src/xmpp/presence.c | 2 +- src/xmpp/session.c | 5 ++++- 10 files changed, 63 insertions(+), 21 deletions(-) diff --git a/src/event/server_events.c b/src/event/server_events.c index f5399ed57..eabf946b3 100644 --- a/src/event/server_events.c +++ b/src/event/server_events.c @@ -646,7 +646,7 @@ sv_ev_incoming_message(ProfMessage* message) if (prefs_get_boolean(PREF_MAM)) { win_print_loading_history(window); - iq_mam_request(chatwin, g_date_time_add_seconds(message->timestamp, 0)); // copy timestamp + iq_mam_request(chatwin, g_date_time_ref(message->timestamp)); // copy timestamp } #ifdef HAVE_OMEMO diff --git a/src/otr/otr.c b/src/otr/otr.c index 5e99575a3..0a67bb03f 100644 --- a/src/otr/otr.c +++ b/src/otr/otr.c @@ -174,6 +174,10 @@ _otr_shutdown(void) free(jid); jid = NULL; } + if (user_state) { + otrl_userstate_free(user_state); + user_state = NULL; + } otrlib_shutdown(); } diff --git a/src/ui/inputwin.c b/src/ui/inputwin.c index 0d2113f01..e1cb10cf0 100644 --- a/src/ui/inputwin.c +++ b/src/ui/inputwin.c @@ -173,8 +173,6 @@ create_input_window(void) char* inp_readline(void) { - free(inp_line); - inp_line = NULL; p_rl_timeout.tv_sec = inp_timeout / 1000; p_rl_timeout.tv_usec = inp_timeout % 1000 * 1000; FD_ZERO(&fds); @@ -216,7 +214,9 @@ inp_readline(void) } } } - return strdup(inp_line); + char* ret = inp_line; + inp_line = NULL; + return ret; } else { return NULL; } diff --git a/src/ui/notifier.c b/src/ui/notifier.c index 469b03d82..344748c32 100644 --- a/src/ui/notifier.c +++ b/src/ui/notifier.c @@ -56,6 +56,9 @@ #include "xmpp/muc.h" static GTimer* remind_timer; +#ifdef HAVE_LIBNOTIFY +static NotifyNotification* notification; +#endif void notifier_initialise(void) @@ -67,6 +70,8 @@ void notifier_uninit(void) { #ifdef HAVE_LIBNOTIFY + g_object_unref(G_OBJECT(notification)); + notification = NULL; if (notify_is_initted()) { notify_uninit(); } @@ -208,7 +213,8 @@ notify(const char* const message, int timeout, const char* const category) notify_init("Profanity"); if (notify_is_initted()) { - NotifyNotification* notification; + if (notification) + g_object_unref(G_OBJECT(notification)); notification = notify_notification_new("Profanity", message, NULL); notify_notification_set_timeout(notification, timeout); notify_notification_set_category(notification, category); diff --git a/src/ui/tray.c b/src/ui/tray.c index a1b286926..c3ab4cf72 100644 --- a/src/ui/tray.c +++ b/src/ui/tray.c @@ -156,12 +156,15 @@ _tray_change_icon(gpointer data) static void _tray_shutdown(void) { - if (gtk_ready && prefs_get_boolean(PREF_TRAY)) { - tray_disable(); + tray_disable(); + if (icon_filename) { + g_string_free(icon_filename, TRUE); + icon_filename = NULL; + } + if (icon_msg_filename) { + g_string_free(icon_msg_filename, TRUE); + icon_msg_filename = NULL; } - g_string_free(icon_filename, TRUE); - g_string_free(icon_msg_filename, TRUE); - gtk_main_quit(); } void @@ -194,7 +197,9 @@ tray_update(void) void tray_set_timer(int interval) { - g_source_remove(timer); + if (timer) { + g_source_remove(timer); + } _tray_change_icon(NULL); timer = g_timeout_add(interval * 1000, _tray_change_icon, NULL); } @@ -219,7 +224,10 @@ void tray_disable(void) { shutting_down = TRUE; - g_source_remove(timer); + if (timer) { + g_source_remove(timer); + timer = 0; + } if (prof_tray) { g_clear_object(&prof_tray); prof_tray = NULL; diff --git a/src/xmpp/connection.c b/src/xmpp/connection.c index c211a6095..7ced154b5 100644 --- a/src/xmpp/connection.c +++ b/src/xmpp/connection.c @@ -169,6 +169,10 @@ connection_shutdown(void) connection_clear_data(); g_hash_table_destroy(conn.requested_features); g_hash_table_destroy(conn.available_resources); + if (conn.sm_state) { + xmpp_free_sm_state(conn.sm_state); + conn.sm_state = NULL; + } if (conn.xmpp_conn) { xmpp_conn_release(conn.xmpp_conn); conn.xmpp_conn = NULL; diff --git a/src/xmpp/iq.c b/src/xmpp/iq.c index c9f66d395..a8a485b2d 100644 --- a/src/xmpp/iq.c +++ b/src/xmpp/iq.c @@ -183,7 +183,7 @@ _iq_handler(xmpp_conn_t* const conn, xmpp_stanza_t* const stanza, void* const us xmpp_stanza_to_text(stanza, &text, &text_size); gboolean cont = plugins_on_iq_stanza_receive(text); xmpp_free(connection_get_ctx(), text); - if (!cont) { + if (!cont || !id_handlers) { return 1; } @@ -261,7 +261,6 @@ iq_handlers_init(void) } received_disco_items = FALSE; - iq_rooms_cache_clear(); iq_handlers_clear(); id_handlers = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)_iq_id_handler_free); @@ -288,6 +287,18 @@ _win_find(char* key, } } +static void +_free_late_delivery_userdata(LateDeliveryUserdata* d) +{ + if (!d) + return; + if (d->enddate) + g_date_time_unref(d->enddate); + if (d->startdate) + g_date_time_unref(d->startdate); + free(d); +} + void iq_handlers_remove_win(ProfWin* window) { @@ -298,9 +309,11 @@ iq_handlers_remove_win(ProfWin* window) while (cur) { LateDeliveryUserdata* del_data = cur->data; next = g_slist_next(cur); - if (del_data->win == (void*)window) + if (del_data->win == (void*)window) { late_delivery_windows = g_slist_delete_link(late_delivery_windows, cur); + _free_late_delivery_userdata(del_data); + } cur = next; } struct iq_win_finder st = { 0 }; @@ -319,10 +332,15 @@ iq_handlers_remove_win(ProfWin* window) void iq_handlers_clear(void) { + iq_rooms_cache_clear(); if (id_handlers) { g_hash_table_destroy(id_handlers); id_handlers = NULL; } + if (late_delivery_windows) { + g_slist_free_full(late_delivery_windows, (GDestroyNotify)_free_late_delivery_userdata); + late_delivery_windows = NULL; + } } static void @@ -2569,8 +2587,7 @@ iq_send_stanza(xmpp_stanza_t* const stanza) if (plugin_text) { xmpp_send_raw_string(conn, "%s", plugin_text); } else { - /* libstrophe does the strlen for us, so simply always pass SIZE_MAX */ - xmpp_send_raw(conn, text, SIZE_MAX); + xmpp_send_raw(conn, text, text_size); } xmpp_free(connection_get_ctx(), text); } @@ -2716,7 +2733,7 @@ void iq_mam_request(ProfChatWin* win, GDateTime* enddate) { ProfMessage* last_msg = log_database_get_limits_info(win->barejid, TRUE); - GDateTime* startdate = g_date_time_add_seconds(last_msg->timestamp, 0); + GDateTime* startdate = g_date_time_ref(last_msg->timestamp); message_free(last_msg); // Save request for later if disco items haven't been received yet diff --git a/src/xmpp/message.c b/src/xmpp/message.c index 5c0477b8c..d96342253 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -1575,7 +1575,7 @@ _send_message_stanza(xmpp_stanza_t* const stanza) if (plugin_text) { xmpp_send_raw_string(conn, "%s", plugin_text); } else { - xmpp_send_raw_string(conn, "%s", text); + xmpp_send_raw(conn, text, text_size); } xmpp_free(connection_get_ctx(), text); } diff --git a/src/xmpp/presence.c b/src/xmpp/presence.c index 62007f142..577a2e823 100644 --- a/src/xmpp/presence.c +++ b/src/xmpp/presence.c @@ -936,7 +936,7 @@ _send_presence_stanza(xmpp_stanza_t* const stanza) if (plugin_text) { xmpp_send_raw_string(conn, "%s", plugin_text); } else { - xmpp_send_raw_string(conn, "%s", text); + xmpp_send_raw(conn, text, text_size); } xmpp_free(connection_get_ctx(), text); } diff --git a/src/xmpp/session.c b/src/xmpp/session.c index 8716bb0d9..75fa891ad 100644 --- a/src/xmpp/session.c +++ b/src/xmpp/session.c @@ -102,6 +102,10 @@ static void _session_free_saved_details(void); static void _session_shutdown(void) { + if (reconnect_timer) { + g_timer_destroy(reconnect_timer); + reconnect_timer = NULL; + } _session_free_internals(); chat_sessions_clear(); @@ -228,7 +232,6 @@ session_disconnect(void) accounts_set_last_activity(session_get_account_name()); - iq_rooms_cache_clear(); iq_handlers_clear(); connection_disconnect(); From 83a4165d92e32abafa4431f6758ea54ded847f47 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Wed, 12 Mar 2025 10:15:35 +0100 Subject: [PATCH 09/13] `plugins_(on|pre)` API now maybe returns NULL. Instead of always returning a `strdup()`'ed version, maybe return NULL and handle this in the calling code. This is still not true for `plugins_pre_chat_message_display()`, where we return the passed `messag` in case there are no plugins, since this would require a bigger refactor of more parts. This also * merges the implementation of `sv_ev_incoming_private_message()` and `sv_ev_delayed_private_message()`, since they differed only by a single line. * untangles the `#ifdef` mess in `cl_ev_send_muc_msg_corrected()`. Signed-off-by: Steffen Jaeckel --- src/event/client_events.c | 88 +++++++++++++++++---------------------- src/event/server_events.c | 39 +++++++---------- src/plugins/plugins.c | 54 ++++++++++++++++++++---- 3 files changed, 98 insertions(+), 83 deletions(-) diff --git a/src/event/client_events.c b/src/event/client_events.c index 9b1f0ffd3..16596c375 100644 --- a/src/event/client_events.c +++ b/src/event/client_events.c @@ -137,9 +137,7 @@ cl_ev_send_msg_correct(ProfChatWin* chatwin, const char* const msg, const char* gboolean request_receipt = prefs_get_boolean(PREF_RECEIPTS_REQUEST); auto_char char* plugin_msg = plugins_pre_chat_message_send(chatwin->barejid, msg); - if (plugin_msg == NULL) { - return; - } + const char* const message = plugin_msg ?: msg; char* replace_id = NULL; if (correct_last_msg) { @@ -148,46 +146,46 @@ cl_ev_send_msg_correct(ProfChatWin* chatwin, const char* const msg, const char* if (chatwin->is_omemo) { #ifdef HAVE_OMEMO - auto_char char* id = omemo_on_message_send((ProfWin*)chatwin, plugin_msg, request_receipt, FALSE, replace_id); + auto_char char* id = omemo_on_message_send((ProfWin*)chatwin, message, request_receipt, FALSE, replace_id); if (id != NULL) { - chat_log_omemo_msg_out(chatwin->barejid, plugin_msg, NULL); - log_database_add_outgoing_chat(id, chatwin->barejid, plugin_msg, replace_id, PROF_MSG_ENC_OMEMO); - chatwin_outgoing_msg(chatwin, plugin_msg, id, PROF_MSG_ENC_OMEMO, request_receipt, replace_id); + chat_log_omemo_msg_out(chatwin->barejid, message, NULL); + log_database_add_outgoing_chat(id, chatwin->barejid, message, replace_id, PROF_MSG_ENC_OMEMO); + chatwin_outgoing_msg(chatwin, message, id, PROF_MSG_ENC_OMEMO, request_receipt, replace_id); } #endif } else if (chatwin->is_ox) { #ifdef HAVE_LIBGPGME // XEP-0373: OpenPGP for XMPP - auto_char char* id = message_send_chat_ox(chatwin->barejid, plugin_msg, request_receipt, replace_id); + auto_char char* id = message_send_chat_ox(chatwin->barejid, message, request_receipt, replace_id); if (id != NULL) { - chat_log_pgp_msg_out(chatwin->barejid, plugin_msg, NULL); - log_database_add_outgoing_chat(id, chatwin->barejid, plugin_msg, replace_id, PROF_MSG_ENC_OX); - chatwin_outgoing_msg(chatwin, plugin_msg, id, PROF_MSG_ENC_OX, request_receipt, replace_id); + chat_log_pgp_msg_out(chatwin->barejid, message, NULL); + log_database_add_outgoing_chat(id, chatwin->barejid, message, replace_id, PROF_MSG_ENC_OX); + chatwin_outgoing_msg(chatwin, message, id, PROF_MSG_ENC_OX, request_receipt, replace_id); } #endif } else if (chatwin->pgp_send) { #ifdef HAVE_LIBGPGME - auto_char char* id = message_send_chat_pgp(chatwin->barejid, plugin_msg, request_receipt, replace_id); + auto_char char* id = message_send_chat_pgp(chatwin->barejid, message, request_receipt, replace_id); if (id != NULL) { - chat_log_pgp_msg_out(chatwin->barejid, plugin_msg, NULL); - log_database_add_outgoing_chat(id, chatwin->barejid, plugin_msg, replace_id, PROF_MSG_ENC_PGP); - chatwin_outgoing_msg(chatwin, plugin_msg, id, PROF_MSG_ENC_PGP, request_receipt, replace_id); + chat_log_pgp_msg_out(chatwin->barejid, message, NULL); + log_database_add_outgoing_chat(id, chatwin->barejid, message, replace_id, PROF_MSG_ENC_PGP); + chatwin_outgoing_msg(chatwin, message, id, PROF_MSG_ENC_PGP, request_receipt, replace_id); } #endif } else { gboolean handled = FALSE; #ifdef HAVE_LIBOTR - handled = otr_on_message_send(chatwin, plugin_msg, request_receipt, replace_id); + handled = otr_on_message_send(chatwin, message, request_receipt, replace_id); #endif if (!handled) { - auto_char char* id = message_send_chat(chatwin->barejid, plugin_msg, oob_url, request_receipt, replace_id); - chat_log_msg_out(chatwin->barejid, plugin_msg, NULL); - log_database_add_outgoing_chat(id, chatwin->barejid, plugin_msg, replace_id, PROF_MSG_ENC_NONE); - chatwin_outgoing_msg(chatwin, plugin_msg, id, PROF_MSG_ENC_NONE, request_receipt, replace_id); + auto_char char* id = message_send_chat(chatwin->barejid, message, oob_url, request_receipt, replace_id); + chat_log_msg_out(chatwin->barejid, message, NULL); + log_database_add_outgoing_chat(id, chatwin->barejid, message, replace_id, PROF_MSG_ENC_NONE); + chatwin_outgoing_msg(chatwin, message, id, PROF_MSG_ENC_NONE, request_receipt, replace_id); } } - plugins_post_chat_message_send(chatwin->barejid, plugin_msg); + plugins_post_chat_message_send(chatwin->barejid, message); return; } @@ -201,9 +199,7 @@ void cl_ev_send_muc_msg_corrected(ProfMucWin* mucwin, const char* const msg, const char* const oob_url, gboolean correct_last_msg) { auto_char char* plugin_msg = plugins_pre_room_message_send(mucwin->roomjid, msg); - if (plugin_msg == NULL) { - return; - } + const char* const message = plugin_msg ?: msg; char* replace_id = NULL; if (correct_last_msg) { @@ -212,30 +208,21 @@ cl_ev_send_muc_msg_corrected(ProfMucWin* mucwin, const char* const msg, const ch #ifdef HAVE_OMEMO if (mucwin->is_omemo) { - auto_char char* id = omemo_on_message_send((ProfWin*)mucwin, plugin_msg, FALSE, TRUE, replace_id); - groupchat_log_omemo_msg_out(mucwin->roomjid, plugin_msg); - log_database_add_outgoing_muc(id, mucwin->roomjid, plugin_msg, replace_id, PROF_MSG_ENC_OMEMO); - mucwin_outgoing_msg(mucwin, plugin_msg, id, PROF_MSG_ENC_OMEMO, replace_id); - } else { - auto_char char* id = message_send_groupchat(mucwin->roomjid, plugin_msg, oob_url, replace_id); - groupchat_log_msg_out(mucwin->roomjid, plugin_msg); - log_database_add_outgoing_muc(id, mucwin->roomjid, plugin_msg, replace_id, PROF_MSG_ENC_NONE); - mucwin_outgoing_msg(mucwin, plugin_msg, id, PROF_MSG_ENC_NONE, replace_id); - } - - plugins_post_room_message_send(mucwin->roomjid, plugin_msg); - return; + auto_char char* id = omemo_on_message_send((ProfWin*)mucwin, message, FALSE, TRUE, replace_id); + groupchat_log_omemo_msg_out(mucwin->roomjid, message); + log_database_add_outgoing_muc(id, mucwin->roomjid, message, replace_id, PROF_MSG_ENC_OMEMO); + mucwin_outgoing_msg(mucwin, message, id, PROF_MSG_ENC_OMEMO, replace_id); + } else #endif + { + auto_char char* id = message_send_groupchat(mucwin->roomjid, message, oob_url, replace_id); + groupchat_log_msg_out(mucwin->roomjid, message); + log_database_add_outgoing_muc(id, mucwin->roomjid, message, replace_id, PROF_MSG_ENC_NONE); + mucwin_outgoing_msg(mucwin, message, id, PROF_MSG_ENC_NONE, replace_id); + } -#ifndef HAVE_OMEMO - auto_char char* id = message_send_groupchat(mucwin->roomjid, plugin_msg, oob_url, replace_id); - groupchat_log_msg_out(mucwin->roomjid, plugin_msg); - log_database_add_outgoing_muc(id, mucwin->roomjid, plugin_msg, replace_id, PROF_MSG_ENC_NONE); - mucwin_outgoing_msg(mucwin, plugin_msg, id, PROF_MSG_ENC_NONE, replace_id); - - plugins_post_room_message_send(mucwin->roomjid, plugin_msg); + plugins_post_room_message_send(mucwin->roomjid, message); return; -#endif } void @@ -253,13 +240,14 @@ cl_ev_send_priv_msg(ProfPrivateWin* privwin, const char* const msg, const char* privwin_message_left_room(privwin); } else { auto_char char* plugin_msg = plugins_pre_priv_message_send(privwin->fulljid, msg); + const char* const message = plugin_msg ?: msg; auto_jid Jid* jidp = jid_create(privwin->fulljid); - auto_char char* id = message_send_private(privwin->fulljid, plugin_msg, oob_url); - chat_log_msg_out(jidp->barejid, plugin_msg, jidp->resourcepart); - log_database_add_outgoing_muc_pm(id, privwin->fulljid, plugin_msg, NULL, PROF_MSG_ENC_NONE); - privwin_outgoing_msg(privwin, plugin_msg); + auto_char char* id = message_send_private(privwin->fulljid, message, oob_url); + chat_log_msg_out(jidp->barejid, message, jidp->resourcepart); + log_database_add_outgoing_muc_pm(id, privwin->fulljid, message, NULL, PROF_MSG_ENC_NONE); + privwin_outgoing_msg(privwin, message); - plugins_post_priv_message_send(privwin->fulljid, plugin_msg); + plugins_post_priv_message_send(privwin->fulljid, message); } } diff --git a/src/event/server_events.c b/src/event/server_events.c index eabf946b3..aa93d9f7e 100644 --- a/src/event/server_events.c +++ b/src/event/server_events.c @@ -345,7 +345,9 @@ sv_ev_room_message(ProfMessage* message) } char* old_plain = message->plain; - message->plain = plugins_pre_room_message_display(message->from_jid->barejid, message->from_jid->resourcepart, message->plain); + auto_char char* plugin_msg = plugins_pre_room_message_display(message->from_jid->barejid, message->from_jid->resourcepart, message->plain); + if (plugin_msg) + message->plain = plugin_msg; GSList* mentions = get_mentions(prefs_get_boolean(PREF_NOTIFY_MENTION_WHOLE_WORD), prefs_get_boolean(PREF_NOTIFY_MENTION_CASE_SENSITIVE), message->plain, mynick); gboolean mention = g_slist_length(mentions) > 0; @@ -409,15 +411,16 @@ sv_ev_room_message(ProfMessage* message) rosterwin_roster(); plugins_post_room_message_display(message->from_jid->barejid, message->from_jid->resourcepart, message->plain); - free(message->plain); message->plain = old_plain; } -void -sv_ev_incoming_private_message(ProfMessage* message) +static void +_sv_ev_private_message(ProfMessage* message) { char* old_plain = message->plain; - message->plain = plugins_pre_priv_message_display(message->from_jid->fulljid, message->plain); + auto_char char* plugin_msg = plugins_pre_priv_message_display(message->from_jid->fulljid, message->plain); + if (plugin_msg) + message->plain = plugin_msg; ProfPrivateWin* privatewin = wins_get_private(message->from_jid->fulljid); if (privatewin == NULL) { @@ -432,32 +435,20 @@ sv_ev_incoming_private_message(ProfMessage* message) plugins_post_priv_message_display(message->from_jid->fulljid, message->plain); - free(message->plain); message->plain = old_plain; +} + +void +sv_ev_incoming_private_message(ProfMessage* message) +{ + _sv_ev_private_message(message); rosterwin_roster(); } void sv_ev_delayed_private_message(ProfMessage* message) { - char* old_plain = message->plain; - message->plain = plugins_pre_priv_message_display(message->from_jid->fulljid, message->plain); - - ProfPrivateWin* privatewin = wins_get_private(message->from_jid->fulljid); - if (privatewin == NULL) { - ProfWin* window = wins_new_private(message->from_jid->fulljid); - privatewin = (ProfPrivateWin*)window; - } - - _clean_incoming_message(message); - privwin_incoming_msg(privatewin, message); - // Intentionally skipping log to DB because we can't authenticate the sender - chat_log_msg_in(message); - - plugins_post_priv_message_display(message->from_jid->fulljid, message->plain); - - free(message->plain); - message->plain = old_plain; + _sv_ev_private_message(message); } void diff --git a/src/plugins/plugins.c b/src/plugins/plugins.c index 7d43f3f41..cb27eb152 100644 --- a/src/plugins/plugins.c +++ b/src/plugins/plugins.c @@ -517,10 +517,14 @@ plugins_on_disconnect(const char* const account_name, const char* const fulljid) char* plugins_pre_chat_message_display(const char* const barejid, const char* const resource, char* message) { + GList* values = g_hash_table_get_values(plugins); + if (!values) { + return message; + } + char* new_message = NULL; char* curr_message = message; - GList* values = g_hash_table_get_values(plugins); GList* curr = values; while (curr) { ProfPlugin* plugin = curr->data; @@ -552,10 +556,14 @@ plugins_post_chat_message_display(const char* const barejid, const char* const r char* plugins_pre_chat_message_send(const char* const barejid, const char* message) { + GList* values = g_hash_table_get_values(plugins); + if (!values) { + return NULL; + } + char* new_message = NULL; char* curr_message = strdup(message); - GList* values = g_hash_table_get_values(plugins); GList* curr = values; while (curr) { ProfPlugin* plugin = curr->data; @@ -595,10 +603,14 @@ plugins_post_chat_message_send(const char* const barejid, const char* message) char* plugins_pre_room_message_display(const char* const barejid, const char* const nick, const char* message) { + GList* values = g_hash_table_get_values(plugins); + if (!values) { + return NULL; + } + char* new_message = NULL; char* curr_message = strdup(message); - GList* values = g_hash_table_get_values(plugins); GList* curr = values; while (curr) { ProfPlugin* plugin = curr->data; @@ -631,10 +643,14 @@ plugins_post_room_message_display(const char* const barejid, const char* const n char* plugins_pre_room_message_send(const char* const barejid, const char* message) { + GList* values = g_hash_table_get_values(plugins); + if (!values) { + return NULL; + } + char* new_message = NULL; char* curr_message = strdup(message); - GList* values = g_hash_table_get_values(plugins); GList* curr = values; while (curr) { ProfPlugin* plugin = curr->data; @@ -697,11 +713,15 @@ plugins_on_room_history_message(const char* const barejid, const char* const nic char* plugins_pre_priv_message_display(const char* const fulljid, const char* message) { + GList* values = g_hash_table_get_values(plugins); + if (!values) { + return NULL; + } + auto_jid Jid* jidp = jid_create(fulljid); char* new_message = NULL; char* curr_message = strdup(message); - GList* values = g_hash_table_get_values(plugins); GList* curr = values; while (curr) { ProfPlugin* plugin = curr->data; @@ -735,11 +755,15 @@ plugins_post_priv_message_display(const char* const fulljid, const char* message char* plugins_pre_priv_message_send(const char* const fulljid, const char* const message) { + GList* values = g_hash_table_get_values(plugins); + if (!values) { + return NULL; + } + auto_jid Jid* jidp = jid_create(fulljid); char* new_message = NULL; char* curr_message = strdup(message); - GList* values = g_hash_table_get_values(plugins); GList* curr = values; while (curr) { ProfPlugin* plugin = curr->data; @@ -782,10 +806,14 @@ plugins_post_priv_message_send(const char* const fulljid, const char* const mess char* plugins_on_message_stanza_send(const char* const text) { + GList* values = g_hash_table_get_values(plugins); + if (!values) { + return NULL; + } + char* new_stanza = NULL; char* curr_stanza = strdup(text); - GList* values = g_hash_table_get_values(plugins); GList* curr = values; while (curr) { ProfPlugin* plugin = curr->data; @@ -825,10 +853,14 @@ plugins_on_message_stanza_receive(const char* const text) char* plugins_on_presence_stanza_send(const char* const text) { + GList* values = g_hash_table_get_values(plugins); + if (!values) { + return NULL; + } + char* new_stanza = NULL; char* curr_stanza = strdup(text); - GList* values = g_hash_table_get_values(plugins); GList* curr = values; while (curr) { ProfPlugin* plugin = curr->data; @@ -868,10 +900,14 @@ plugins_on_presence_stanza_receive(const char* const text) char* plugins_on_iq_stanza_send(const char* const text) { + GList* values = g_hash_table_get_values(plugins); + if (!values) { + return NULL; + } + char* new_stanza = NULL; char* curr_stanza = strdup(text); - GList* values = g_hash_table_get_values(plugins); GList* curr = values; while (curr) { ProfPlugin* plugin = curr->data; From 79dfe2bba2d911be03ab92554c210123c52f4264 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 7 Mar 2025 22:32:18 +0100 Subject: [PATCH 10/13] Don't error-out if Valgrind fails Signed-off-by: Steffen Jaeckel --- Dockerfile.arch | 1 + ci-build.sh | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Dockerfile.arch b/Dockerfile.arch index fa2afb871..d8d981e5c 100644 --- a/Dockerfile.arch +++ b/Dockerfile.arch @@ -9,6 +9,7 @@ RUN pacman -Syu --noconfirm && pacman -S --needed --noconfirm \ cmake \ cmocka \ curl \ + debuginfod \ doxygen \ expat \ gcc \ diff --git a/ci-build.sh b/ci-build.sh index 963db462e..77e0eb352 100755 --- a/ci-build.sh +++ b/ci-build.sh @@ -65,6 +65,7 @@ case "$ARCH" in "--without-xscreensaver" "--disable-gdk-pixbuf" "") + source /etc/profile.d/debuginfod.sh 2>/dev/null || true ;; darwin*) tests=( @@ -127,7 +128,7 @@ case "$ARCH" in ./configure ${tests[0]} --enable-valgrind $* $MAKE CC="${CC}" - $MAKE check-valgrind + $MAKE check-valgrind || log_content ./test-suite-memcheck.log $MAKE distclean ;; esac From ef476cbf968d7e1457d8b69375f03275e57e63de Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 7 Mar 2025 22:35:50 +0100 Subject: [PATCH 11/13] Cancel all running build jobs on force-push of a PR branch Signed-off-by: Steffen Jaeckel --- .github/workflows/main.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 406b58fda..94915218a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -6,6 +6,10 @@ on: pull_request: branches: [master] +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: linux: runs-on: ubuntu-latest From 00d50d457c33ae633885a020220d6697b35ccf7c Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Mon, 10 Mar 2025 12:25:55 +0100 Subject: [PATCH 12/13] Let's use Debian as reference Valgrind platform Signed-off-by: Steffen Jaeckel --- ci-build.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ci-build.sh b/ci-build.sh index 77e0eb352..82ff76ffc 100755 --- a/ci-build.sh +++ b/ci-build.sh @@ -128,7 +128,11 @@ case "$ARCH" in ./configure ${tests[0]} --enable-valgrind $* $MAKE CC="${CC}" - $MAKE check-valgrind || log_content ./test-suite-memcheck.log + if grep '^ID=' /etc/os-release | grep -q -e debian; then + $MAKE check-valgrind + else + $MAKE check-valgrind || log_content ./test-suite-memcheck.log + fi $MAKE distclean ;; esac From edb41bef60f68a74f26d156daaa880cb8f485525 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Mon, 10 Mar 2025 15:46:59 +0100 Subject: [PATCH 13/13] Re-factor notifier. * Add an internal `_notifier_uninit()` function. * Instead of `#ifdef` inside `notify()`, provide different target-specific static implementations. This also introduces `log_error_once()`. Signed-off-by: Steffen Jaeckel --- src/log.h | 15 +++ src/ui/notifier.c | 237 ++++++++++++++++++++++++++-------------------- 2 files changed, 148 insertions(+), 104 deletions(-) diff --git a/src/log.h b/src/log.h index 14394a243..d40d1889b 100644 --- a/src/log.h +++ b/src/log.h @@ -38,6 +38,10 @@ #include +#define PROF_TMPVAR__(n, l) n##l +#define PROF_TMPVAR_(n, l) PROF_TMPVAR__(n, l) +#define PROF_TMPVAR(n) PROF_TMPVAR_(PROF_##n##_, __LINE__) + // log levels typedef enum { PROF_LEVEL_DEBUG, @@ -58,6 +62,17 @@ void log_msg(log_level_t level, const char* const area, const char* const msg); int log_level_from_string(char* log_level, log_level_t* level); const char* log_string_from_level(log_level_t level); +#define _log_Xtype_once(type, ...) \ + do { \ + static gboolean PROF_TMPVAR(once) = FALSE; \ + if (!PROF_TMPVAR(once)) { \ + log_##type(__VA_ARGS__); \ + PROF_TMPVAR(once) = TRUE; \ + } \ + } while (0) + +#define log_error_once(...) _log_Xtype_once(error, __VA_ARGS__) + void log_stderr_init(log_level_t level); void log_stderr_handler(void); diff --git a/src/ui/notifier.c b/src/ui/notifier.c index 344748c32..32c975907 100644 --- a/src/ui/notifier.c +++ b/src/ui/notifier.c @@ -56,8 +56,139 @@ #include "xmpp/muc.h" static GTimer* remind_timer; + #ifdef HAVE_LIBNOTIFY static NotifyNotification* notification; + +static void +_notifier_uninit(void) +{ + if (notify_is_initted()) { + g_object_unref(G_OBJECT(notification)); + notification = NULL; + notify_uninit(); + } +} +#else +static void +_notifier_uninit(void) +{ +} +#endif + +#ifdef HAVE_LIBNOTIFY +static void +_notify(const char* const message, int timeout, const char* const category) +{ + log_debug("Attempting notification: %s", message); + if (notify_is_initted()) { + log_debug("Reinitialising libnotify"); + notify_uninit(); + } else { + log_debug("Initialising libnotify"); + } + notify_init("Profanity"); + + if (notify_is_initted()) { + if (notification) + g_object_unref(G_OBJECT(notification)); + notification = notify_notification_new("Profanity", message, NULL); + notify_notification_set_timeout(notification, timeout); + notify_notification_set_category(notification, category); + notify_notification_set_urgency(notification, NOTIFY_URGENCY_NORMAL); + + GError* error = NULL; + gboolean notify_success = notify_notification_show(notification, &error); + + if (!notify_success) { + log_error("Error sending desktop notification:"); + log_error(" -> Message : %s", message); + log_error(" -> Error : %s", error->message); + g_error_free(error); + } else { + log_debug("Notification sent."); + } + } else { + log_error("Libnotify not initialised."); + } +} +#elif defined(PLATFORM_CYGWIN) +static void +_notify(const char* const message, int timeout, const char* const category) +{ + NOTIFYICONDATA nid; + memset(&nid, 0, sizeof(nid)); + nid.cbSize = sizeof(NOTIFYICONDATA); + // nid.hWnd = hWnd; + nid.uID = 100; + nid.uVersion = NOTIFYICON_VERSION; + // nid.uCallbackMessage = WM_MYMESSAGE; + nid.hIcon = LoadIcon(NULL, IDI_APPLICATION); + strcpy(nid.szTip, "Tray Icon"); + nid.uFlags = NIF_MESSAGE | NIF_ICON | NIF_TIP; + Shell_NotifyIcon(NIM_ADD, &nid); + + // For a Ballon Tip + nid.uFlags = NIF_INFO; + strcpy(nid.szInfoTitle, "Profanity"); // Title + strncpy(nid.szInfo, message, sizeof(nid.szInfo) - 1); // Copy Tip + nid.uTimeout = timeout; // 3 Seconds + nid.dwInfoFlags = NIIF_INFO; + + Shell_NotifyIcon(NIM_MODIFY, &nid); +} +#elif defined(HAVE_OSXNOTIFY) +static void +_notify(const char* const message, int timeout, const char* const category) +{ + GString* notify_command = g_string_new("terminal-notifier -title \"Profanity\" -message '"); + + auto_char char* escaped_single = str_replace(message, "'", "'\\''"); + + if (escaped_single[0] == '<') { + g_string_append(notify_command, "\\<"); + g_string_append(notify_command, &escaped_single[1]); + } else if (escaped_single[0] == '[') { + g_string_append(notify_command, "\\["); + g_string_append(notify_command, &escaped_single[1]); + } else if (escaped_single[0] == '(') { + g_string_append(notify_command, "\\("); + g_string_append(notify_command, &escaped_single[1]); + } else if (escaped_single[0] == '{') { + g_string_append(notify_command, "\\{"); + g_string_append(notify_command, &escaped_single[1]); + } else { + g_string_append(notify_command, escaped_single); + } + + g_string_append(notify_command, "'"); + + char* term_name = getenv("TERM_PROGRAM"); + char* app_id = NULL; + if (g_strcmp0(term_name, "Apple_Terminal") == 0) { + app_id = "com.apple.Terminal"; + } else if (g_strcmp0(term_name, "iTerm.app") == 0) { + app_id = "com.googlecode.iterm2"; + } + + if (app_id) { + g_string_append(notify_command, " -sender "); + g_string_append(notify_command, app_id); + } + + int res = system(notify_command->str); + if (res == -1) { + log_error("Could not send desktop notification."); + } + + g_string_free(notify_command, TRUE); +} +#else +static void +_notify(const char* const message, int timeout, const char* const category) +{ + log_error_once("Notification backend missing"); +} #endif void @@ -69,13 +200,7 @@ notifier_initialise(void) void notifier_uninit(void) { -#ifdef HAVE_LIBNOTIFY - g_object_unref(G_OBJECT(notification)); - notification = NULL; - if (notify_is_initted()) { - notify_uninit(); - } -#endif + _notifier_uninit(); g_timer_destroy(remind_timer); } @@ -202,102 +327,6 @@ notify_remind(void) void notify(const char* const message, int timeout, const char* const category) { -#ifdef HAVE_LIBNOTIFY log_debug("Attempting notification: %s", message); - if (notify_is_initted()) { - log_debug("Reinitialising libnotify"); - notify_uninit(); - } else { - log_debug("Initialising libnotify"); - } - notify_init("Profanity"); - - if (notify_is_initted()) { - if (notification) - g_object_unref(G_OBJECT(notification)); - notification = notify_notification_new("Profanity", message, NULL); - notify_notification_set_timeout(notification, timeout); - notify_notification_set_category(notification, category); - notify_notification_set_urgency(notification, NOTIFY_URGENCY_NORMAL); - - GError* error = NULL; - gboolean notify_success = notify_notification_show(notification, &error); - - if (!notify_success) { - log_error("Error sending desktop notification:"); - log_error(" -> Message : %s", message); - log_error(" -> Error : %s", error->message); - g_error_free(error); - } else { - log_debug("Notification sent."); - } - } else { - log_error("Libnotify not initialised."); - } -#endif -#ifdef PLATFORM_CYGWIN - NOTIFYICONDATA nid; - memset(&nid, 0, sizeof(nid)); - nid.cbSize = sizeof(NOTIFYICONDATA); - // nid.hWnd = hWnd; - nid.uID = 100; - nid.uVersion = NOTIFYICON_VERSION; - // nid.uCallbackMessage = WM_MYMESSAGE; - nid.hIcon = LoadIcon(NULL, IDI_APPLICATION); - strcpy(nid.szTip, "Tray Icon"); - nid.uFlags = NIF_MESSAGE | NIF_ICON | NIF_TIP; - Shell_NotifyIcon(NIM_ADD, &nid); - - // For a Ballon Tip - nid.uFlags = NIF_INFO; - strcpy(nid.szInfoTitle, "Profanity"); // Title - strncpy(nid.szInfo, message, sizeof(nid.szInfo) - 1); // Copy Tip - nid.uTimeout = timeout; // 3 Seconds - nid.dwInfoFlags = NIIF_INFO; - - Shell_NotifyIcon(NIM_MODIFY, &nid); -#endif -#ifdef HAVE_OSXNOTIFY - GString* notify_command = g_string_new("terminal-notifier -title \"Profanity\" -message '"); - - auto_char char* escaped_single = str_replace(message, "'", "'\\''"); - - if (escaped_single[0] == '<') { - g_string_append(notify_command, "\\<"); - g_string_append(notify_command, &escaped_single[1]); - } else if (escaped_single[0] == '[') { - g_string_append(notify_command, "\\["); - g_string_append(notify_command, &escaped_single[1]); - } else if (escaped_single[0] == '(') { - g_string_append(notify_command, "\\("); - g_string_append(notify_command, &escaped_single[1]); - } else if (escaped_single[0] == '{') { - g_string_append(notify_command, "\\{"); - g_string_append(notify_command, &escaped_single[1]); - } else { - g_string_append(notify_command, escaped_single); - } - - g_string_append(notify_command, "'"); - - char* term_name = getenv("TERM_PROGRAM"); - char* app_id = NULL; - if (g_strcmp0(term_name, "Apple_Terminal") == 0) { - app_id = "com.apple.Terminal"; - } else if (g_strcmp0(term_name, "iTerm.app") == 0) { - app_id = "com.googlecode.iterm2"; - } - - if (app_id) { - g_string_append(notify_command, " -sender "); - g_string_append(notify_command, app_id); - } - - int res = system(notify_command->str); - if (res == -1) { - log_error("Could not send desktop notification."); - } - - g_string_free(notify_command, TRUE); -#endif + _notify(message, timeout, category); }