From 847a86de50a5a2afd319bfaef94b2dbef6f39934 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Tue, 14 Nov 2023 14:53:49 +0100 Subject: [PATCH 1/8] add `connection_get_jid()` Use a singleton `Jid` inside the connection instead of always re-creating a `Jid` from the same string. Signed-off-by: Steffen Jaeckel --- src/chatlog.c | 27 ++++++++-------- src/command/cmd_funcs.c | 9 ++---- src/database.c | 18 ++++------- src/event/client_events.c | 3 +- src/event/server_events.c | 3 +- src/omemo/omemo.c | 20 ++++-------- src/tools/editor.c | 2 +- src/ui/chatwin.c | 2 +- src/ui/vcardwin.c | 3 +- src/ui/window.c | 3 +- src/xmpp/connection.c | 53 ++++++++++++++++++++++---------- src/xmpp/message.c | 11 ++----- src/xmpp/muc.c | 3 +- src/xmpp/omemo.c | 21 ++++--------- src/xmpp/roster.c | 3 +- src/xmpp/xmpp.h | 3 +- tests/unittests/xmpp/stub_xmpp.c | 5 +++ 17 files changed, 88 insertions(+), 101 deletions(-) diff --git a/src/chatlog.c b/src/chatlog.c index 01aa4401..9956dadd 100644 --- a/src/chatlog.c +++ b/src/chatlog.c @@ -85,8 +85,7 @@ void chat_log_msg_out(const char* const barejid, const char* const msg, const char* const resource) { if (prefs_get_boolean(PREF_CHLOG)) { - auto_char char* mybarejid = connection_get_barejid(); - _chat_log_chat(mybarejid, barejid, msg, PROF_OUT_LOG, NULL, resource); + _chat_log_chat(connection_get_barejid(), barejid, msg, PROF_OUT_LOG, NULL, resource); } } @@ -94,7 +93,7 @@ void chat_log_otr_msg_out(const char* const barejid, const char* const msg, const char* const resource) { if (prefs_get_boolean(PREF_CHLOG)) { - auto_char char* mybarejid = connection_get_barejid(); + const char* mybarejid = connection_get_barejid(); auto_gchar gchar* pref_otr_log = prefs_get_string(PREF_OTR_LOG); if (strcmp(pref_otr_log, "on") == 0) { _chat_log_chat(mybarejid, barejid, msg, PROF_OUT_LOG, NULL, resource); @@ -108,7 +107,7 @@ void chat_log_pgp_msg_out(const char* const barejid, const char* const msg, const char* const resource) { if (prefs_get_boolean(PREF_CHLOG)) { - auto_char char* mybarejid = connection_get_barejid(); + const char* mybarejid = connection_get_barejid(); auto_gchar gchar* pref_pgp_log = prefs_get_string(PREF_PGP_LOG); if (strcmp(pref_pgp_log, "on") == 0) { _chat_log_chat(mybarejid, barejid, msg, PROF_OUT_LOG, NULL, resource); @@ -122,7 +121,7 @@ void chat_log_omemo_msg_out(const char* const barejid, const char* const msg, const char* const resource) { if (prefs_get_boolean(PREF_CHLOG)) { - auto_char char* mybarejid = connection_get_barejid(); + const char* mybarejid = connection_get_barejid(); auto_gchar gchar* pref_omemo_log = prefs_get_string(PREF_OMEMO_LOG); if (strcmp(pref_omemo_log, "on") == 0) { _chat_log_chat(mybarejid, barejid, msg, PROF_OUT_LOG, NULL, resource); @@ -136,7 +135,7 @@ void chat_log_otr_msg_in(ProfMessage* message) { if (prefs_get_boolean(PREF_CHLOG)) { - auto_char char* mybarejid = connection_get_barejid(); + const char* mybarejid = connection_get_barejid(); auto_gchar gchar* pref_otr_log = prefs_get_string(PREF_OTR_LOG); if (message->enc == PROF_MSG_ENC_NONE || (strcmp(pref_otr_log, "on") == 0)) { if (message->type == PROF_MSG_TYPE_MUCPM) { @@ -158,7 +157,7 @@ void chat_log_pgp_msg_in(ProfMessage* message) { if (prefs_get_boolean(PREF_CHLOG)) { - auto_char char* mybarejid = connection_get_barejid(); + const char* mybarejid = connection_get_barejid(); auto_gchar gchar* pref_pgp_log = prefs_get_string(PREF_PGP_LOG); if (strcmp(pref_pgp_log, "on") == 0) { if (message->type == PROF_MSG_TYPE_MUCPM) { @@ -180,7 +179,7 @@ void chat_log_omemo_msg_in(ProfMessage* message) { if (prefs_get_boolean(PREF_CHLOG)) { - auto_char char* mybarejid = connection_get_barejid(); + const char* mybarejid = connection_get_barejid(); auto_gchar gchar* pref_omemo_log = prefs_get_string(PREF_OMEMO_LOG); if (strcmp(pref_omemo_log, "on") == 0) { if (message->type == PROF_MSG_TYPE_MUCPM) { @@ -202,7 +201,7 @@ void chat_log_msg_in(ProfMessage* message) { if (prefs_get_boolean(PREF_CHLOG)) { - auto_char char* mybarejid = connection_get_barejid(); + const char* mybarejid = connection_get_barejid(); if (message->type == PROF_MSG_TYPE_MUCPM) { _chat_log_chat(mybarejid, message->from_jid->barejid, message->plain, PROF_IN_LOG, message->timestamp, message->from_jid->resourcepart); @@ -308,9 +307,8 @@ void groupchat_log_msg_out(const gchar* const room, const gchar* const msg) { if (prefs_get_boolean(PREF_GRLOG)) { - auto_char char* mybarejid = connection_get_barejid(); char* mynick = muc_nick(room); - _groupchat_log_chat(mybarejid, room, mynick, msg); + _groupchat_log_chat(connection_get_barejid(), room, mynick, msg); } } @@ -318,8 +316,7 @@ void groupchat_log_msg_in(const gchar* const room, const gchar* const nick, const gchar* const msg) { if (prefs_get_boolean(PREF_GRLOG)) { - auto_char char* mybarejid = connection_get_barejid(); - _groupchat_log_chat(mybarejid, room, nick, msg); + _groupchat_log_chat(connection_get_barejid(), room, nick, msg); } } @@ -327,7 +324,7 @@ void groupchat_log_omemo_msg_out(const gchar* const room, const gchar* const msg) { if (prefs_get_boolean(PREF_CHLOG)) { - auto_char char* mybarejid = connection_get_barejid(); + const char* mybarejid = connection_get_barejid(); auto_gchar gchar* pref_omemo_log = prefs_get_string(PREF_OMEMO_LOG); char* mynick = muc_nick(room); @@ -343,7 +340,7 @@ void groupchat_log_omemo_msg_in(const gchar* const room, const gchar* const nick, const gchar* const msg) { if (prefs_get_boolean(PREF_CHLOG)) { - auto_char char* mybarejid = connection_get_barejid(); + const char* mybarejid = connection_get_barejid(); auto_gchar gchar* pref_omemo_log = prefs_get_string(PREF_OMEMO_LOG); if (strcmp(pref_omemo_log, "on") == 0) { diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 15b818a7..b13115db 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -3448,12 +3448,11 @@ cmd_caps(ProfWin* window, const char* const command, gchar** args) static void _send_software_version_iq_to_fulljid(char* request) { - auto_char char* mybarejid = connection_get_barejid(); auto_jid Jid* jid = jid_create(request); if (jid == NULL || jid->fulljid == NULL) { cons_show("You must provide a full jid to the /software command."); - } else if (g_strcmp0(jid->barejid, mybarejid) == 0) { + } else if (g_strcmp0(jid->barejid, connection_get_barejid()) == 0) { cons_show("Cannot request software version for yourself."); } else { iq_send_software_version(jid->fulljid); @@ -4842,8 +4841,7 @@ cmd_disco(ProfWin* window, const char* const command, gchar** args) if (args[1]) { jid = g_string_append(jid, args[1]); } else { - auto_jid Jid* jidp = jid_create(connection_get_fulljid()); - jid = g_string_append(jid, jidp->domainpart); + jid = g_string_append(jid, connection_get_jid()->domainpart); } if (g_strcmp0(args[0], "info") == 0) { @@ -5039,8 +5037,7 @@ cmd_lastactivity(ProfWin* window, const char* const command, gchar** args) if ((g_strcmp0(args[0], "get") == 0)) { if (args[1] == NULL) { - auto_jid Jid* jidp = jid_create(connection_get_fulljid()); - GString* jid = g_string_new(jidp->domainpart); + GString* jid = g_string_new(connection_get_jid()->domainpart); iq_last_activity_request(jid->str); diff --git a/src/database.c b/src/database.c index ce1d1d3d..a8e3c7c0 100644 --- a/src/database.c +++ b/src/database.c @@ -235,9 +235,7 @@ log_database_add_incoming(ProfMessage* message) if (message->to_jid) { _add_to_db(message, NULL, message->from_jid, message->to_jid); } else { - auto_jid Jid* myjid = jid_create(connection_get_fulljid()); - - _add_to_db(message, NULL, message->from_jid, myjid); + _add_to_db(message, NULL, message->from_jid, connection_get_jid()); } } @@ -253,9 +251,7 @@ _log_database_add_outgoing(char* type, const char* const id, const char* const b msg->timestamp = g_date_time_new_now_local(); // TODO: get from outside. best to have whole ProfMessage from outside msg->enc = enc; - auto_jid Jid* myjid = jid_create(connection_get_fulljid()); - - _add_to_db(msg, type, myjid, msg->from_jid); // TODO: myjid now in profmessage + _add_to_db(msg, type, connection_get_jid(), msg->from_jid); // TODO: myjid now in profmessage message_free(msg); } @@ -283,9 +279,8 @@ ProfMessage* log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_last) { sqlite3_stmt* stmt = NULL; - const char* jid = connection_get_fulljid(); - auto_jid Jid* myjid = jid_create(jid); - if (!myjid) + const Jid* myjid = connection_get_jid(); + if (!myjid->str) return NULL; const char* order = is_last ? "DESC" : "ASC"; @@ -327,9 +322,8 @@ GSList* log_database_get_previous_chat(const gchar* const contact_barejid, const char* start_time, char* end_time, gboolean from_start, gboolean flip) { sqlite3_stmt* stmt = NULL; - const char* jid = connection_get_fulljid(); - auto_jid Jid* myjid = jid_create(jid); - if (!myjid) + const Jid* myjid = connection_get_jid(); + if (!myjid->str) return NULL; // Flip order when querying older pages diff --git a/src/event/client_events.c b/src/event/client_events.c index bccedc97..a37e7b5b 100644 --- a/src/event/client_events.c +++ b/src/event/client_events.c @@ -86,8 +86,7 @@ cl_ev_connect_account(ProfAccount* account) void cl_ev_disconnect(void) { - auto_char char* mybarejid = connection_get_barejid(); - cons_show("%s logged out successfully.", mybarejid); + cons_show("%s logged out successfully.", connection_get_barejid()); ui_close_all_wins(); ev_disconnect_cleanup(); diff --git a/src/event/server_events.c b/src/event/server_events.c index 1638cc7f..d7ea97cd 100644 --- a/src/event/server_events.c +++ b/src/event/server_events.c @@ -630,8 +630,7 @@ sv_ev_incoming_message(ProfMessage* message) char* looking_for_jid = message->from_jid->barejid; if (message->is_mam) { - auto_char char* mybarejid = connection_get_barejid(); - if (g_strcmp0(mybarejid, message->from_jid->barejid) == 0) { + if (g_strcmp0(connection_get_barejid(), message->from_jid->barejid) == 0) { if (message->to_jid) { looking_for_jid = message->to_jid->barejid; } diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index d908795b..d7880dcc 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -339,7 +339,7 @@ omemo_publish_crypto_materials(void) static void _acquire_sender_devices_list(void) { - auto_char char* barejid = connection_get_barejid(); + const char* barejid = connection_get_barejid(); g_hash_table_insert(omemo_ctx.device_list_handler, strdup(barejid), _handle_own_device_list); omemo_devicelist_request(barejid); @@ -375,8 +375,7 @@ omemo_start_session(const char* const barejid) log_debug("[OMEMO] missing device list for %s", barejid); // Own devices are handled by _handle_own_device_list // We won't add _handle_device_list_start_session for ourself - auto_char char* mybarejid = connection_get_barejid(); - if (g_strcmp0(mybarejid, barejid) != 0) { + if (g_strcmp0(connection_get_barejid(), barejid) != 0) { g_hash_table_insert(omemo_ctx.device_list_handler, strdup(barejid), _handle_device_list_start_session); } omemo_devicelist_request(barejid); @@ -680,7 +679,7 @@ omemo_on_message_send(ProfWin* win, const char* const message, gboolean request_ { char* id = NULL; int res; - auto_jid Jid* jid = jid_create(connection_get_fulljid()); + const Jid* jid = connection_get_jid(); GList* keys = NULL; unsigned char* key; @@ -754,15 +753,12 @@ omemo_on_message_send(ProfWin* win, const char* const message, gboolean request_ // Don't encrypt for this device (according to // ). // Yourself as recipients in case of MUC - char* mybarejid = connection_get_barejid(); - if (!g_strcmp0(mybarejid, recipients_iter->data)) { + if (!g_strcmp0(connection_get_barejid(), recipients_iter->data)) { if (GPOINTER_TO_INT(device_ids_iter->data) == omemo_ctx.device_id) { - free(mybarejid); log_debug("[OMEMO][SEND] Skipping %d (my device) ", GPOINTER_TO_INT(device_ids_iter->data)); continue; } } - free(mybarejid); log_debug("[OMEMO][SEND] recipients with device id %d for %s", GPOINTER_TO_INT(device_ids_iter->data), recipients_iter->data); res = session_cipher_create(&cipher, omemo_ctx.store, &address, omemo_ctx.signal); @@ -1806,14 +1802,10 @@ out: char* omemo_qrcode_str() { - char* mybarejid = connection_get_barejid(); - char* fingerprint = omemo_own_fingerprint(FALSE); + auto_char char* fingerprint = omemo_own_fingerprint(FALSE); uint32_t sid = omemo_device_id(); - char* qrstr = g_strdup_printf("xmpp:%s?omemo-sid-%d=%s", mybarejid, sid, fingerprint); - - free(mybarejid); - free(fingerprint); + char* qrstr = g_strdup_printf("xmpp:%s?omemo-sid-%d=%s", connection_get_barejid(), sid, fingerprint); return qrstr; } diff --git a/src/tools/editor.c b/src/tools/editor.c index 4cd70d9d..8f90da9b 100644 --- a/src/tools/editor.c +++ b/src/tools/editor.c @@ -56,7 +56,7 @@ get_message_from_editor(gchar* message, gchar** returned_message) auto_gchar gchar* filename = NULL; GError* glib_error = NULL; - auto_char char* jid = connection_get_barejid(); + const char* jid = connection_get_barejid(); if (jid) { filename = files_file_in_account_data_path(DIR_EDITOR, jid, "compose.md"); } else { diff --git a/src/ui/chatwin.c b/src/ui/chatwin.c index 4317cc78..cf0b675f 100644 --- a/src/ui/chatwin.c +++ b/src/ui/chatwin.c @@ -427,7 +427,7 @@ chatwin_outgoing_msg(ProfChatWin* chatwin, const char* const message, char* id, auto_char char* enc_char = get_enc_char(enc_mode, chatwin->outgoing_char); - auto_jid Jid* myjid = jid_create(connection_get_fulljid()); + const Jid* myjid = connection_get_jid(); auto_char char* display_message = plugins_pre_chat_message_display(myjid->barejid, myjid->resourcepart, strdup(message)); if (request_receipt && id) { diff --git a/src/ui/vcardwin.c b/src/ui/vcardwin.c index 0cef89fd..4ee8c96a 100644 --- a/src/ui/vcardwin.c +++ b/src/ui/vcardwin.c @@ -53,8 +53,7 @@ gchar* vcardwin_get_string(ProfVcardWin* vcardwin) { GString* string = g_string_new("vCard: "); - auto_char char* jid = connection_get_barejid(); - g_string_append(string, jid); + g_string_append(string, connection_get_barejid()); if (vcardwin->vcard && vcardwin->vcard->modified) { g_string_append(string, " (modified)"); diff --git a/src/ui/window.c b/src/ui/window.c index b0452091..2faa925e 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -373,9 +373,8 @@ win_get_title(ProfWin* window) assert(vcardwin->memcheck == PROFVCARDWIN_MEMCHECK); GString* title = g_string_new("vCard "); - auto_char char* jid = connection_get_barejid(); - g_string_append(title, jid); + g_string_append(title, connection_get_barejid()); if (vcardwin->vcard->modified) { g_string_append(title, " *"); diff --git a/src/xmpp/connection.c b/src/xmpp/connection.c index 71e7d21d..759fded8 100644 --- a/src/xmpp/connection.c +++ b/src/xmpp/connection.c @@ -68,6 +68,7 @@ typedef struct prof_conn_t char* presence_message; int priority; char* domain; + Jid* jid; GHashTable* available_resources; GHashTable* features_by_jid; GHashTable* requested_features; @@ -135,6 +136,7 @@ connection_init(void) conn.conn_last_event = XMPP_CONN_DISCONNECT; conn.presence_message = NULL; conn.domain = NULL; + conn.jid = NULL; conn.features_by_jid = NULL; conn.available_resources = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)resource_destroy); conn.requested_features = g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL); @@ -562,6 +564,9 @@ connection_disconnect(void) g_free(prof_identifier); prof_identifier = NULL; + + jid_destroy(conn.jid); + conn.jid = NULL; } void @@ -724,17 +729,34 @@ connection_get_fulljid(void) } } -char* +const Jid* +connection_get_jid(void) +{ + static const char* fulljid; + const char* cur_fulljid = connection_get_fulljid(); + if (!conn.jid || cur_fulljid != fulljid) { + fulljid = cur_fulljid; + if (conn.jid) + jid_destroy(conn.jid); + conn.jid = jid_create(fulljid); + if (!conn.jid) { + log_error("Could not create connection-wide JID object from \"%s\"", STR_MAYBE_NULL(fulljid)); + /* In case we failed to create the jid or we're not yet + * connected (or whatever else could fail) return the + * pointer to a zero-initialized static object, so + * de-referencing that pointer won't fail. + */ + static const Jid jid = { 0 }; + return &jid; + } + } + return conn.jid; +} + +const char* connection_get_barejid(void) { - const char* jid = connection_get_fulljid(); - if (!jid) - return NULL; - - auto_jid Jid* jidp = jid_create(jid); - char* result = strdup(jidp->barejid); - - return result; + return connection_get_jid()->barejid; } char* @@ -953,9 +975,8 @@ _connection_handler(xmpp_conn_t* const xmpp_conn, const xmpp_conn_event_t status log_debug("Connection handler: XMPP_CONN_CONNECT"); conn.conn_status = JABBER_CONNECTED; - Jid* my_jid = jid_create(xmpp_conn_get_jid(conn.xmpp_conn)); - conn.domain = strdup(my_jid->domainpart); - jid_destroy(my_jid); + connection_get_jid(); + conn.domain = strdup(conn.jid->domainpart); connection_clear_data(); conn.features_by_jid = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_hash_table_destroy); @@ -979,10 +1000,10 @@ _connection_handler(xmpp_conn_t* const xmpp_conn, const xmpp_conn_event_t status log_debug("Connection handler: XMPP_CONN_RAW_CONNECT"); conn.conn_status = JABBER_RAW_CONNECTED; - Jid* my_raw_jid = jid_create(xmpp_conn_get_jid(conn.xmpp_conn)); - log_debug("jid: %s", xmpp_conn_get_jid(conn.xmpp_conn)); - conn.domain = strdup(my_raw_jid->domainpart); - jid_destroy(my_raw_jid); + connection_get_jid(); + conn.jid = jid_create(xmpp_conn_get_jid(conn.xmpp_conn)); + log_debug("jid: %s", conn.jid->str); + conn.domain = strdup(conn.jid->domainpart); connection_clear_data(); conn.features_by_jid = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_hash_table_destroy); diff --git a/src/xmpp/message.c b/src/xmpp/message.c index 867a7d0c..3f8addbc 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -101,9 +101,7 @@ static GHashTable* pubsub_event_handlers; gchar* get_display_name(const ProfMessage* const message, int* flags) { - auto_char char* barejid = connection_get_barejid(); - - if (g_strcmp0(barejid, message->from_jid->barejid) == 0) { + if (g_strcmp0(connection_get_barejid(), message->from_jid->barejid) == 0) { return g_strdup("me"); } else { if (flags) @@ -261,11 +259,10 @@ _message_handler(xmpp_conn_t* const conn, xmpp_stanza_t* const stanza, void* con if (carbons) { // carbon must come from ourselves - auto_char char* mybarejid = connection_get_barejid(); const char* const stanza_from = xmpp_stanza_get_from(stanza); if (stanza_from) { - if (g_strcmp0(mybarejid, stanza_from) != 0) { + if (g_strcmp0(connection_get_barejid(), stanza_from) != 0) { log_warning("Invalid carbon received, from: %s", stanza_from); msg_stanza = NULL; } else { @@ -1464,10 +1461,8 @@ _handle_chat(xmpp_stanza_t* const stanza, gboolean is_mam, gboolean is_carbon, c if (message->plain || message->body || message->encrypted) { if (is_carbon) { - auto_char char* mybarejid = connection_get_barejid(); - // if we are the recipient, treat as standard incoming message - if (g_strcmp0(mybarejid, message->to_jid->barejid) == 0) { + if (g_strcmp0(connection_get_barejid(), message->to_jid->barejid) == 0) { sv_ev_incoming_carbon(message); // else treat as a sent message } else { diff --git a/src/xmpp/muc.c b/src/xmpp/muc.c index bbcd23aa..34ee6b65 100644 --- a/src/xmpp/muc.c +++ b/src/xmpp/muc.c @@ -882,8 +882,7 @@ muc_members_add(const char* const room, const char* const jid) if (g_hash_table_insert(chat_room->members, strdup(jid), NULL)) { #ifdef HAVE_OMEMO if (chat_room->anonymity_type == MUC_ANONYMITY_TYPE_NONANONYMOUS) { - auto_char char* our_barejid = connection_get_barejid(); - if (strcmp(jid, our_barejid) != 0) { + if (strcmp(jid, connection_get_barejid()) != 0) { omemo_start_session(jid); } } diff --git a/src/xmpp/omemo.c b/src/xmpp/omemo.c index cc15b9db..459b5419 100644 --- a/src/xmpp/omemo.c +++ b/src/xmpp/omemo.c @@ -87,9 +87,8 @@ omemo_devicelist_configure(void) { xmpp_ctx_t* const ctx = connection_get_ctx(); auto_char char* id = connection_create_stanza_id(); - auto_jid Jid* jid = jid_create(connection_get_fulljid()); - xmpp_stanza_t* iq = stanza_create_pubsub_configure_request(ctx, id, jid->barejid, STANZA_NS_OMEMO_DEVICELIST); + xmpp_stanza_t* iq = stanza_create_pubsub_configure_request(ctx, id, connection_get_jid()->barejid, STANZA_NS_OMEMO_DEVICELIST); iq_id_handler_add(id, _omemo_devicelist_configure_submit, NULL, NULL); @@ -196,9 +195,7 @@ omemo_start_device_session_handle_bundle(xmpp_stanza_t* const stanza, void* cons } if (!from_attr) { - Jid* jid = jid_create(connection_get_fulljid()); - from = strdup(jid->barejid); - jid_destroy(jid); + from = strdup(connection_get_jid()->barejid); } else { from = strdup(from_attr); } @@ -608,9 +605,8 @@ _omemo_devicelist_configure_submit(xmpp_stanza_t* const stanza, void* const user form_set_value(form, "pubsub#access_model", "open"); xmpp_ctx_t* const ctx = connection_get_ctx(); - auto_jid Jid* jid = jid_create(connection_get_fulljid()); auto_char char* id = connection_create_stanza_id(); - xmpp_stanza_t* iq = stanza_create_pubsub_configure_submit(ctx, id, jid->barejid, STANZA_NS_OMEMO_DEVICELIST, form); + xmpp_stanza_t* iq = stanza_create_pubsub_configure_submit(ctx, id, connection_get_jid()->barejid, STANZA_NS_OMEMO_DEVICELIST, form); iq_id_handler_add(id, _omemo_devicelist_configure_result, NULL, NULL); @@ -633,10 +629,7 @@ _omemo_devicelist_configure_result(xmpp_stanza_t* const stanza, void* const user log_debug("[OMEMO] node configured"); // Try to publish - char* barejid = connection_get_barejid(); - omemo_devicelist_request(barejid); - - free(barejid); + omemo_devicelist_request(connection_get_barejid()); return 0; } @@ -660,12 +653,11 @@ _omemo_bundle_publish_result(xmpp_stanza_t* const stanza, void* const userdata) log_debug("[OMEMO] cannot publish bundle with open access model, trying to configure node"); xmpp_ctx_t* const ctx = connection_get_ctx(); - auto_jid Jid* jid = jid_create(connection_get_fulljid()); auto_char char* id = connection_create_stanza_id(); auto_gchar gchar* node = g_strdup_printf("%s:%d", STANZA_NS_OMEMO_BUNDLES, omemo_device_id()); log_debug("[OMEMO] node: %s", node); - xmpp_stanza_t* iq = stanza_create_pubsub_configure_request(ctx, id, jid->barejid, node); + xmpp_stanza_t* iq = stanza_create_pubsub_configure_request(ctx, id, connection_get_jid()->barejid, node); iq_id_handler_add(id, _omemo_bundle_publish_configure, NULL, userdata); @@ -700,10 +692,9 @@ _omemo_bundle_publish_configure(xmpp_stanza_t* const stanza, void* const userdat form_set_value(form, "pubsub#access_model", "open"); xmpp_ctx_t* const ctx = connection_get_ctx(); - auto_jid Jid* jid = jid_create(connection_get_fulljid()); auto_char char* id = connection_create_stanza_id(); auto_gchar gchar* node = g_strdup_printf("%s:%d", STANZA_NS_OMEMO_BUNDLES, omemo_device_id()); - xmpp_stanza_t* iq = stanza_create_pubsub_configure_submit(ctx, id, jid->barejid, node, form); + xmpp_stanza_t* iq = stanza_create_pubsub_configure_submit(ctx, id, connection_get_jid()->barejid, node, form); iq_id_handler_add(id, _omemo_bundle_publish_configure_result, NULL, userdata); diff --git a/src/xmpp/roster.c b/src/xmpp/roster.c index 2ef36d78..9d2b3972 100644 --- a/src/xmpp/roster.c +++ b/src/xmpp/roster.c @@ -200,9 +200,8 @@ roster_set_handler(xmpp_stanza_t* const stanza) } // if from attribute exists and it is not current users barejid, ignore push - auto_char char* mybarejid = connection_get_barejid(); const char* from = xmpp_stanza_get_from(stanza); - if (from && (strcmp(from, mybarejid) != 0)) { + if (from && (strcmp(from, connection_get_barejid()) != 0)) { log_warning("Received alleged roster push from: %s", from); return; } diff --git a/src/xmpp/xmpp.h b/src/xmpp/xmpp.h index 494d5fc1..bbff1928 100644 --- a/src/xmpp/xmpp.h +++ b/src/xmpp/xmpp.h @@ -193,7 +193,8 @@ jabber_conn_status_t connection_get_status(void); char* connection_get_presence_msg(void); void connection_set_presence_msg(const char* const message); const char* connection_get_fulljid(void); -char* connection_get_barejid(void); +const Jid* connection_get_jid(void); +const char* connection_get_barejid(void); char* connection_get_user(void); char* connection_create_uuid(void); void connection_free_uuid(char* uuid); diff --git a/tests/unittests/xmpp/stub_xmpp.c b/tests/unittests/xmpp/stub_xmpp.c index ffa7565d..9d616b08 100644 --- a/tests/unittests/xmpp/stub_xmpp.c +++ b/tests/unittests/xmpp/stub_xmpp.c @@ -58,6 +58,11 @@ void connection_disconnect(void) { } +const Jid* +connection_get_jid(void) +{ + return mock_ptr_type(Jid*); +} const char* connection_get_fulljid(void) { From 835ea397ac2a05c0af1416e90498f9934b94271f Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Tue, 14 Nov 2023 14:55:29 +0100 Subject: [PATCH 2/8] Tidy up some code Signed-off-by: Steffen Jaeckel --- src/common.c | 4 ++-- src/event/server_events.c | 3 +-- src/plugins/plugins.c | 4 +--- src/ui/console.c | 3 +-- src/xmpp/connection.c | 4 +--- src/xmpp/session.c | 4 +--- 6 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/common.c b/src/common.c index 1f235d58..be065a49 100644 --- a/src/common.c +++ b/src/common.c @@ -652,7 +652,7 @@ format_call_external_argv(const char* template, const char* url, const char* fil return argv; } -gchar* +static gchar* _unique_filename(const char* filename) { gchar* unique = g_strdup(filename); @@ -676,7 +676,7 @@ _unique_filename(const char* filename) return unique; } -bool +static bool _has_directory_suffix(const char* path) { return (g_str_has_suffix(path, ".") diff --git a/src/event/server_events.c b/src/event/server_events.c index d7ea97cd..e759088e 100644 --- a/src/event/server_events.c +++ b/src/event/server_events.c @@ -190,8 +190,7 @@ sv_ev_roster_received(void) connection_set_presence_msg(status_message); cl_ev_presence_send(conn_presence, diff_secs); - const char* fulljid = connection_get_fulljid(); - plugins_on_connect(account_name, fulljid); + plugins_on_connect(account_name, connection_get_fulljid()); } void diff --git a/src/plugins/plugins.c b/src/plugins/plugins.c index 274b711e..c64dceef 100644 --- a/src/plugins/plugins.c +++ b/src/plugins/plugins.c @@ -252,9 +252,7 @@ plugins_load(const char* const name, GString* error_message) if (plugin) { g_hash_table_insert(plugins, strdup(name), plugin); if (connection_get_status() == JABBER_CONNECTED) { - const char* account_name = session_get_account_name(); - const char* fulljid = connection_get_fulljid(); - plugin->init_func(plugin, PACKAGE_VERSION, PACKAGE_STATUS, account_name, fulljid); + plugin->init_func(plugin, PACKAGE_VERSION, PACKAGE_STATUS, session_get_account_name(), connection_get_fulljid()); } else { plugin->init_func(plugin, PACKAGE_VERSION, PACKAGE_STATUS, NULL, NULL); } diff --git a/src/ui/console.c b/src/ui/console.c index 370fa5bb..dfbbfc14 100644 --- a/src/ui/console.c +++ b/src/ui/console.c @@ -458,8 +458,7 @@ cons_show_login_success(ProfAccount* account, gboolean secured) { ProfWin* console = wins_get_console(); - const char* fulljid = connection_get_fulljid(); - win_print(console, THEME_DEFAULT, "-", "%s logged in successfully, ", fulljid); + win_print(console, THEME_DEFAULT, "-", "%s logged in successfully, ", connection_get_fulljid()); resource_presence_t presence = accounts_get_login_presence(account->name); const char* presence_str = string_from_resource_presence(presence); diff --git a/src/xmpp/connection.c b/src/xmpp/connection.c index 759fded8..b199eb05 100644 --- a/src/xmpp/connection.c +++ b/src/xmpp/connection.c @@ -836,9 +836,7 @@ connection_create_stanza_id(void) (guchar*)prof_identifier, strlen(prof_identifier), rndid, strlen(rndid)); - char* ret = g_strdup_printf("%s%s", rndid, hmac); - - return ret; + return g_strdup_printf("%s%s", rndid, hmac); } char* diff --git a/src/xmpp/session.c b/src/xmpp/session.c index 05c35e87..5e6ae09d 100644 --- a/src/xmpp/session.c +++ b/src/xmpp/session.c @@ -209,9 +209,7 @@ session_disconnect(void) if (connection_get_status() == JABBER_CONNECTED) { log_info("Closing connection"); - char* account_name = session_get_account_name(); - const char* fulljid = connection_get_fulljid(); - plugins_on_disconnect(account_name, fulljid); + plugins_on_disconnect(session_get_account_name(), connection_get_fulljid()); accounts_set_last_activity(session_get_account_name()); From fdfe3e2ad96a4d401bb6263749057a4a14a9799c Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Thu, 16 Nov 2023 13:24:17 +0100 Subject: [PATCH 3/8] Rework OMEMO handling on disconnect * Fix missing destruction of `session_store` and mutex * Replace `glib_hash_table_free()` The glib API `g_hash_table_destroy()` does exactly the same. * Use the default libsignal `destroy_func` instead of doing that manually * Set internal state to `0` after everything is cleaned up Signed-off-by: Steffen Jaeckel --- src/common.c | 7 ----- src/common.h | 1 - src/omemo/omemo.c | 79 ++++++++++++++++++++++++----------------------- src/omemo/omemo.h | 2 -- src/omemo/store.c | 6 ++-- 5 files changed, 44 insertions(+), 51 deletions(-) diff --git a/src/common.c b/src/common.c index be065a49..cffb908d 100644 --- a/src/common.c +++ b/src/common.c @@ -774,13 +774,6 @@ unique_filename_from_url(const char* url, const char* path) return unique_filename; } -void -glib_hash_table_free(GHashTable* hash_table) -{ - g_hash_table_remove_all(hash_table); - g_hash_table_unref(hash_table); -} - /* build profanity version string. * example: 0.13.1dev.master.69d8c1f9 */ diff --git a/src/common.h b/src/common.h index 83260fe6..5996c5d5 100644 --- a/src/common.h +++ b/src/common.h @@ -193,7 +193,6 @@ gchar** format_call_external_argv(const char* template, const char* url, const c gchar* unique_filename_from_url(const char* url, const char* path); gchar* get_expanded_path(const char* path); -void glib_hash_table_free(GHashTable* hash_table); char* basename_from_url(const char* url); gchar* prof_get_version(void); diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index d7880dcc..0382de04 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -66,8 +66,6 @@ #define AESGCM_URL_NONCE_LEN (2 * OMEMO_AESGCM_NONCE_LENGTH) #define AESGCM_URL_KEY_LEN (2 * OMEMO_AESGCM_KEY_LENGTH) -static gboolean loaded; - static void _generate_pre_keys(int count); static void _generate_signed_pre_key(void); static gboolean _load_identity(void); @@ -87,10 +85,8 @@ static void _acquire_sender_devices_list(void); typedef gboolean (*OmemoDeviceListHandler)(const char* const jid, GList* device_list); -struct omemo_context_t +typedef struct omemo_context { - pthread_mutexattr_t attr; - pthread_mutex_t lock; signal_context* signal; uint32_t device_id; GHashTable* device_list; @@ -108,11 +104,18 @@ struct omemo_context_t prof_keyfile_t sessions; prof_keyfile_t knowndevices; GHashTable* known_devices; - GHashTable* fingerprint_ac; -}; + gboolean loaded; +} omemo_context; static omemo_context omemo_ctx; +static struct omemo_static_data +{ + pthread_mutexattr_t attr; + pthread_mutex_t lock; + GHashTable* fingerprint_ac; +} omemo_static_data; + void omemo_init(void) { @@ -121,20 +124,21 @@ omemo_init(void) cons_show("Error initializing OMEMO crypto: gcry_check_version() failed"); } - pthread_mutexattr_init(&omemo_ctx.attr); - pthread_mutexattr_settype(&omemo_ctx.attr, PTHREAD_MUTEX_RECURSIVE); - pthread_mutex_init(&omemo_ctx.lock, &omemo_ctx.attr); + pthread_mutexattr_init(&omemo_static_data.attr); + pthread_mutexattr_settype(&omemo_static_data.attr, PTHREAD_MUTEX_RECURSIVE); + pthread_mutex_init(&omemo_static_data.lock, &omemo_static_data.attr); - omemo_ctx.fingerprint_ac = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)autocomplete_free); + 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_ctx.fingerprint_ac) { - g_hash_table_destroy(omemo_ctx.fingerprint_ac); - omemo_ctx.fingerprint_ac = NULL; + 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 @@ -181,7 +185,7 @@ omemo_on_connect(ProfAccount* account) .contains_session_func = contains_session, .delete_session_func = delete_session, .delete_all_sessions_func = delete_all_sessions, - .destroy_func = NULL, + .destroy_func = (void (*)(void*))g_hash_table_destroy, .user_data = omemo_ctx.session_store }; signal_protocol_store_context_set_session_store(omemo_ctx.store, &session_store); @@ -192,7 +196,7 @@ omemo_on_connect(ProfAccount* account) .store_pre_key = store_pre_key, .contains_pre_key = contains_pre_key, .remove_pre_key = remove_pre_key, - .destroy_func = NULL, + .destroy_func = (void (*)(void*))g_hash_table_destroy, .user_data = omemo_ctx.pre_key_store }; signal_protocol_store_context_set_pre_key_store(omemo_ctx.store, &pre_key_store); @@ -203,7 +207,7 @@ omemo_on_connect(ProfAccount* account) .store_signed_pre_key = store_signed_pre_key, .contains_signed_pre_key = contains_signed_pre_key, .remove_signed_pre_key = remove_signed_pre_key, - .destroy_func = NULL, + .destroy_func = (void (*)(void*))g_hash_table_destroy, .user_data = omemo_ctx.signed_pre_key_store }; signal_protocol_store_context_set_signed_pre_key_store(omemo_ctx.store, &signed_pre_key_store); @@ -219,10 +223,10 @@ omemo_on_connect(ProfAccount* account) }; signal_protocol_store_context_set_identity_key_store(omemo_ctx.store, &identity_key_store); - loaded = FALSE; + omemo_ctx.loaded = FALSE; omemo_ctx.device_list = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_list_free); omemo_ctx.device_list_handler = g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL); - omemo_ctx.known_devices = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)glib_hash_table_free); + omemo_ctx.known_devices = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_hash_table_destroy); auto_gchar gchar* omemo_dir = files_file_in_account_data_path(DIR_OMEMO, account->jid, NULL); if (!omemo_dir) { @@ -253,15 +257,13 @@ omemo_on_connect(ProfAccount* account) void omemo_on_disconnect(void) { - if (!loaded) { + if (!omemo_ctx.loaded) { return; } - glib_hash_table_free(omemo_ctx.signed_pre_key_store); - glib_hash_table_free(omemo_ctx.pre_key_store); - glib_hash_table_free(omemo_ctx.known_devices); - glib_hash_table_free(omemo_ctx.device_list_handler); - glib_hash_table_free(omemo_ctx.device_list); + g_hash_table_destroy(omemo_ctx.known_devices); + g_hash_table_destroy(omemo_ctx.device_list_handler); + g_hash_table_destroy(omemo_ctx.device_list); free_keyfile(&omemo_ctx.knowndevices); free_keyfile(&omemo_ctx.sessions); @@ -276,12 +278,13 @@ omemo_on_disconnect(void) ec_public_key_destroy((signal_type_base*)pub); signal_context_destroy(omemo_ctx.signal); + memset(&omemo_ctx, 0, sizeof(omemo_ctx)); } void omemo_generate_crypto_materials(ProfAccount* account) { - if (loaded) { + if (omemo_ctx.loaded) { return; } @@ -316,7 +319,7 @@ omemo_generate_crypto_materials(ProfAccount* account) omemo_identity_keyfile_save(); - loaded = TRUE; + omemo_ctx.loaded = TRUE; omemo_publish_crypto_materials(); omemo_start_sessions(); @@ -327,7 +330,7 @@ omemo_publish_crypto_materials(void) { log_debug("[OMEMO] publish crypto materials"); - if (loaded != TRUE) { + if (omemo_ctx.loaded != TRUE) { cons_show("OMEMO: cannot publish crypto materials before they are generated"); log_error("[OMEMO] cannot publish crypto materials before they are generated"); return; @@ -404,7 +407,7 @@ omemo_start_muc_sessions(const char* const roomjid) gboolean omemo_loaded(void) { - return loaded; + return omemo_ctx.loaded; } uint32_t @@ -1261,15 +1264,13 @@ omemo_untrust(const char* const jid, const char* const fingerprint_formatted) static void _lock(void* user_data) { - omemo_context* ctx = (omemo_context*)user_data; - pthread_mutex_lock(&ctx->lock); + pthread_mutex_lock(&omemo_static_data.lock); } static void _unlock(void* user_data) { - omemo_context* ctx = (omemo_context*)user_data; - pthread_mutex_unlock(&ctx->lock); + pthread_mutex_unlock(&omemo_static_data.lock); } static void @@ -1337,7 +1338,7 @@ omemo_key_free(omemo_key_t* key) char* omemo_fingerprint_autocomplete(const char* const search_str, gboolean previous, void* context) { - Autocomplete ac = g_hash_table_lookup(omemo_ctx.fingerprint_ac, context); + Autocomplete ac = g_hash_table_lookup(omemo_static_data.fingerprint_ac, context); if (ac != NULL) { return autocomplete_complete(ac, search_str, FALSE, previous); } else { @@ -1348,9 +1349,11 @@ omemo_fingerprint_autocomplete(const char* const search_str, gboolean previous, void omemo_fingerprint_autocomplete_reset(void) { + if (!omemo_static_data.fingerprint_ac) + return; gpointer value; GHashTableIter iter; - g_hash_table_iter_init(&iter, omemo_ctx.fingerprint_ac); + g_hash_table_iter_init(&iter, omemo_static_data.fingerprint_ac); while (g_hash_table_iter_next(&iter, NULL, &value)) { Autocomplete ac = value; @@ -1510,7 +1513,7 @@ _load_identity(void) _generate_signed_pre_key(); } - loaded = TRUE; + omemo_ctx.loaded = TRUE; omemo_identity_keyfile_save(); omemo_start_sessions(); @@ -1622,10 +1625,10 @@ _cache_device_identity(const char* const jid, uint32_t device_id, ec_public_key* g_key_file_set_string(omemo_ctx.knowndevices.keyfile, jid, device_id_str, fingerprint); omemo_known_devices_keyfile_save(); - Autocomplete ac = g_hash_table_lookup(omemo_ctx.fingerprint_ac, jid); + Autocomplete ac = g_hash_table_lookup(omemo_static_data.fingerprint_ac, jid); if (ac == NULL) { ac = autocomplete_new(); - g_hash_table_insert(omemo_ctx.fingerprint_ac, strdup(jid), ac); + g_hash_table_insert(omemo_static_data.fingerprint_ac, strdup(jid), ac); } char* formatted_fingerprint = omemo_format_fingerprint(fingerprint); diff --git a/src/omemo/omemo.h b/src/omemo/omemo.h index 624bec51..040ab4b7 100644 --- a/src/omemo/omemo.h +++ b/src/omemo/omemo.h @@ -51,8 +51,6 @@ typedef enum { PROF_OMEMOPOLICY_ALWAYS } prof_omemopolicy_t; -typedef struct omemo_context_t omemo_context; - typedef struct omemo_key { unsigned char* data; diff --git a/src/omemo/store.c b/src/omemo/store.c index 08291460..fd085a58 100644 --- a/src/omemo/store.c +++ b/src/omemo/store.c @@ -43,7 +43,7 @@ GHashTable* session_store_new(void) { - return g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)glib_hash_table_free); + return g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_hash_table_destroy); } GHashTable* @@ -61,7 +61,7 @@ signed_pre_key_store_new(void) void identity_key_store_new(identity_key_store_t* identity_key_store) { - identity_key_store->trusted = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)glib_hash_table_free); + identity_key_store->trusted = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_hash_table_destroy); identity_key_store->private = NULL; identity_key_store->public = NULL; } @@ -71,7 +71,7 @@ identity_key_store_destroy(identity_key_store_t* identity_key_store) { signal_buffer_bzero_free(identity_key_store->private); signal_buffer_bzero_free(identity_key_store->public); - glib_hash_table_free(identity_key_store->trusted); + g_hash_table_destroy(identity_key_store->trusted); } int From bac24601da20904eb61f9f257f7fc9e4cd816adc Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Tue, 5 Dec 2023 12:59:51 +0100 Subject: [PATCH 4/8] Introduce `equals_our_barejid()` Instead of always repeating the same pattern, introduce a helper function. Signed-off-by: Steffen Jaeckel --- src/command/cmd_funcs.c | 2 +- src/event/server_events.c | 2 +- src/omemo/omemo.c | 4 ++-- src/xmpp/connection.c | 6 ++++++ src/xmpp/message.c | 6 +++--- src/xmpp/muc.c | 2 +- src/xmpp/roster.c | 2 +- src/xmpp/xmpp.h | 1 + tests/unittests/xmpp/stub_xmpp.c | 6 ++++++ 9 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index b13115db..ac06c92d 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -3452,7 +3452,7 @@ _send_software_version_iq_to_fulljid(char* request) if (jid == NULL || jid->fulljid == NULL) { cons_show("You must provide a full jid to the /software command."); - } else if (g_strcmp0(jid->barejid, connection_get_barejid()) == 0) { + } else if (equals_our_barejid(jid->barejid)) { cons_show("Cannot request software version for yourself."); } else { iq_send_software_version(jid->fulljid); diff --git a/src/event/server_events.c b/src/event/server_events.c index e759088e..c79f990a 100644 --- a/src/event/server_events.c +++ b/src/event/server_events.c @@ -629,7 +629,7 @@ sv_ev_incoming_message(ProfMessage* message) char* looking_for_jid = message->from_jid->barejid; if (message->is_mam) { - if (g_strcmp0(connection_get_barejid(), message->from_jid->barejid) == 0) { + if (equals_our_barejid(message->from_jid->barejid)) { if (message->to_jid) { looking_for_jid = message->to_jid->barejid; } diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index 0382de04..53e7791c 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -378,7 +378,7 @@ omemo_start_session(const char* const barejid) log_debug("[OMEMO] missing device list for %s", barejid); // Own devices are handled by _handle_own_device_list // We won't add _handle_device_list_start_session for ourself - if (g_strcmp0(connection_get_barejid(), barejid) != 0) { + if (equals_our_barejid(barejid)) { g_hash_table_insert(omemo_ctx.device_list_handler, strdup(barejid), _handle_device_list_start_session); } omemo_devicelist_request(barejid); @@ -756,7 +756,7 @@ omemo_on_message_send(ProfWin* win, const char* const message, gboolean request_ // Don't encrypt for this device (according to // ). // Yourself as recipients in case of MUC - if (!g_strcmp0(connection_get_barejid(), recipients_iter->data)) { + if (equals_our_barejid(recipients_iter->data)) { if (GPOINTER_TO_INT(device_ids_iter->data) == omemo_ctx.device_id) { log_debug("[OMEMO][SEND] Skipping %d (my device) ", GPOINTER_TO_INT(device_ids_iter->data)); continue; diff --git a/src/xmpp/connection.c b/src/xmpp/connection.c index b199eb05..49e82296 100644 --- a/src/xmpp/connection.c +++ b/src/xmpp/connection.c @@ -759,6 +759,12 @@ connection_get_barejid(void) return connection_get_jid()->barejid; } +gboolean +equals_our_barejid(const char* cmp) +{ + return g_strcmp0(connection_get_jid()->barejid, cmp) == 0; +} + char* connection_get_user(void) { diff --git a/src/xmpp/message.c b/src/xmpp/message.c index 3f8addbc..93ef260c 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -101,7 +101,7 @@ static GHashTable* pubsub_event_handlers; gchar* get_display_name(const ProfMessage* const message, int* flags) { - if (g_strcmp0(connection_get_barejid(), message->from_jid->barejid) == 0) { + if (equals_our_barejid(message->from_jid->barejid)) { return g_strdup("me"); } else { if (flags) @@ -262,7 +262,7 @@ _message_handler(xmpp_conn_t* const conn, xmpp_stanza_t* const stanza, void* con const char* const stanza_from = xmpp_stanza_get_from(stanza); if (stanza_from) { - if (g_strcmp0(connection_get_barejid(), stanza_from) != 0) { + if (!equals_our_barejid(stanza_from)) { log_warning("Invalid carbon received, from: %s", stanza_from); msg_stanza = NULL; } else { @@ -1462,7 +1462,7 @@ _handle_chat(xmpp_stanza_t* const stanza, gboolean is_mam, gboolean is_carbon, c if (message->plain || message->body || message->encrypted) { if (is_carbon) { // if we are the recipient, treat as standard incoming message - if (g_strcmp0(connection_get_barejid(), message->to_jid->barejid) == 0) { + if (equals_our_barejid(message->to_jid->barejid)) { sv_ev_incoming_carbon(message); // else treat as a sent message } else { diff --git a/src/xmpp/muc.c b/src/xmpp/muc.c index 34ee6b65..6e4f33fc 100644 --- a/src/xmpp/muc.c +++ b/src/xmpp/muc.c @@ -882,7 +882,7 @@ muc_members_add(const char* const room, const char* const jid) if (g_hash_table_insert(chat_room->members, strdup(jid), NULL)) { #ifdef HAVE_OMEMO if (chat_room->anonymity_type == MUC_ANONYMITY_TYPE_NONANONYMOUS) { - if (strcmp(jid, connection_get_barejid()) != 0) { + if (!equals_our_barejid(jid)) { omemo_start_session(jid); } } diff --git a/src/xmpp/roster.c b/src/xmpp/roster.c index 9d2b3972..dac0d735 100644 --- a/src/xmpp/roster.c +++ b/src/xmpp/roster.c @@ -201,7 +201,7 @@ roster_set_handler(xmpp_stanza_t* const stanza) // if from attribute exists and it is not current users barejid, ignore push const char* from = xmpp_stanza_get_from(stanza); - if (from && (strcmp(from, connection_get_barejid()) != 0)) { + if (!equals_our_barejid(from)) { log_warning("Received alleged roster push from: %s", from); return; } diff --git a/src/xmpp/xmpp.h b/src/xmpp/xmpp.h index bbff1928..57079d1f 100644 --- a/src/xmpp/xmpp.h +++ b/src/xmpp/xmpp.h @@ -195,6 +195,7 @@ void connection_set_presence_msg(const char* const message); const char* connection_get_fulljid(void); const Jid* connection_get_jid(void); const char* connection_get_barejid(void); +gboolean equals_our_barejid(const char* cmp); char* connection_get_user(void); char* connection_create_uuid(void); void connection_free_uuid(char* uuid); diff --git a/tests/unittests/xmpp/stub_xmpp.c b/tests/unittests/xmpp/stub_xmpp.c index 9d616b08..4d6bd3df 100644 --- a/tests/unittests/xmpp/stub_xmpp.c +++ b/tests/unittests/xmpp/stub_xmpp.c @@ -75,6 +75,12 @@ connection_get_barejid(void) return mock_ptr_type(char*); } +gboolean +equals_our_barejid(const char* cmp) +{ + return TRUE; +} + char* connection_get_user(void) { From b4c088232eb298757472bd2e52a7074181cf2933 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Sat, 9 Dec 2023 15:08:47 +0100 Subject: [PATCH 5/8] Improve const correctness Signed-off-by: Steffen Jaeckel --- src/command/cmd_funcs.c | 17 +++++------------ src/config/accounts.c | 2 +- src/event/client_events.c | 3 +-- src/xmpp/connection.c | 18 +++++------------- src/xmpp/connection.h | 2 +- src/xmpp/iq.c | 16 ++++++++-------- src/xmpp/iq.h | 4 ++-- src/xmpp/presence.c | 6 +++--- src/xmpp/session.c | 2 +- src/xmpp/xmpp.h | 14 +++++++------- tests/unittests/xmpp/stub_xmpp.c | 20 ++++++++++---------- 11 files changed, 44 insertions(+), 60 deletions(-) diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index ac06c92d..cd169169 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -5037,17 +5037,11 @@ cmd_lastactivity(ProfWin* window, const char* const command, gchar** args) if ((g_strcmp0(args[0], "get") == 0)) { if (args[1] == NULL) { - GString* jid = g_string_new(connection_get_jid()->domainpart); - - iq_last_activity_request(jid->str); - - g_string_free(jid, TRUE); - - return TRUE; + iq_last_activity_request(connection_get_jid()->domainpart); } else { iq_last_activity_request(args[1]); - return TRUE; } + return TRUE; } cons_bad_cmd_usage(command); @@ -8330,7 +8324,7 @@ cmd_command_list(ProfWin* window, const char* const command, gchar** args) return TRUE; } - char* jid = args[1]; + const char* jid = args[1]; if (jid == NULL) { switch (window->type) { case WIN_MUC: @@ -8391,7 +8385,7 @@ cmd_command_exec(ProfWin* window, const char* const command, gchar** args) return TRUE; } - char* jid = args[2]; + const char* jid = args[2]; if (jid == NULL) { switch (window->type) { case WIN_MUC: @@ -9668,12 +9662,11 @@ cmd_change_password(ProfWin* window, const char* const command, gchar** args) return TRUE; } - auto_char char* user = connection_get_user(); auto_char char* passwd = ui_ask_password(false); auto_char char* confirm_passwd = ui_ask_password(true); if (g_strcmp0(passwd, confirm_passwd) == 0) { - iq_register_change_password(user, passwd); + iq_register_change_password(connection_get_user(), passwd); } else { cons_show("Aborted! The new password and the confirmed password do not match."); } diff --git a/src/config/accounts.c b/src/config/accounts.c index 16edc219..5f1e5d37 100644 --- a/src/config/accounts.c +++ b/src/config/accounts.c @@ -223,7 +223,7 @@ accounts_get_account(const char* const name) } else { jabber_conn_status_t conn_status = connection_get_status(); if (conn_status == JABBER_CONNECTED) { - char* conf_jid = connection_jid_for_feature(XMPP_FEATURE_MUC); + const char* conf_jid = connection_jid_for_feature(XMPP_FEATURE_MUC); if (conf_jid) { muc_service = strdup(conf_jid); } diff --git a/src/event/client_events.c b/src/event/client_events.c index a37e7b5b..4bbcad32 100644 --- a/src/event/client_events.c +++ b/src/event/client_events.c @@ -122,8 +122,7 @@ cl_ev_presence_send(const resource_presence_t presence_type, const int idle_secs char* account_name = session_get_account_name(); ProfAccount* account = accounts_get_account(account_name); if (account->pgp_keyid) { - char* msg = connection_get_presence_msg(); - signed_status = p_gpg_sign(msg, account->pgp_keyid); + signed_status = p_gpg_sign(connection_get_presence_msg(), account->pgp_keyid); } account_free(account); #endif diff --git a/src/xmpp/connection.c b/src/xmpp/connection.c index 49e82296..a3eabc82 100644 --- a/src/xmpp/connection.c +++ b/src/xmpp/connection.c @@ -648,7 +648,7 @@ connection_supports(const char* const feature) return ret; } -char* +const char* connection_jid_for_feature(const char* const feature) { if (conn.features_by_jid == NULL) { @@ -765,18 +765,10 @@ equals_our_barejid(const char* cmp) return g_strcmp0(connection_get_jid()->barejid, cmp) == 0; } -char* +const char* connection_get_user(void) { - const char* jid = connection_get_fulljid(); - if (!jid) - return NULL; - char* result = strdup(jid); - - char* split = strchr(result, '@'); - *split = '\0'; - - return result; + return connection_get_jid()->localpart; } void @@ -845,13 +837,13 @@ connection_create_stanza_id(void) return g_strdup_printf("%s%s", rndid, hmac); } -char* +const char* connection_get_domain(void) { return conn.domain; } -char* +const char* connection_get_presence_msg(void) { return conn.presence_message; diff --git a/src/xmpp/connection.h b/src/xmpp/connection.h index d4ade03a..897a9fbc 100644 --- a/src/xmpp/connection.h +++ b/src/xmpp/connection.h @@ -56,7 +56,7 @@ void connection_set_disco_items(GSList* items); xmpp_conn_t* connection_get_conn(void); xmpp_ctx_t* connection_get_ctx(void); -char* connection_get_domain(void); +const char* connection_get_domain(void); void connection_request_features(void); void connection_features_received(const char* const jid); GHashTable* connection_get_features(const char* const jid); diff --git a/src/xmpp/iq.c b/src/xmpp/iq.c index cd310cc3..f5aed560 100644 --- a/src/xmpp/iq.c +++ b/src/xmpp/iq.c @@ -417,7 +417,7 @@ iq_rooms_cache_clear(void) } void -iq_room_list_request(gchar* conferencejid, gchar* filter) +iq_room_list_request(const char* conferencejid, char* filter) { if (g_hash_table_contains(rooms_cache, conferencejid)) { log_debug("Rooms request cached for: %s", conferencejid); @@ -466,7 +466,7 @@ iq_disable_carbons(void) void iq_http_upload_request(HTTPUpload* upload) { - char* jid = connection_jid_for_feature(STANZA_NS_HTTP_UPLOAD); + const char* jid = connection_jid_for_feature(STANZA_NS_HTTP_UPLOAD); if (jid == NULL) { cons_show_error("XEP-0363 HTTP File Upload is not supported by the server"); return; @@ -484,7 +484,7 @@ iq_http_upload_request(HTTPUpload* upload) } void -iq_disco_info_request(gchar* jid) +iq_disco_info_request(const char* jid) { xmpp_ctx_t* const ctx = connection_get_ctx(); auto_char char* id = connection_create_stanza_id(); @@ -497,7 +497,7 @@ iq_disco_info_request(gchar* jid) } void -iq_disco_info_request_onconnect(gchar* jid) +iq_disco_info_request_onconnect(const char* jid) { xmpp_ctx_t* const ctx = connection_get_ctx(); auto_char char* id = connection_create_stanza_id(); @@ -510,7 +510,7 @@ iq_disco_info_request_onconnect(gchar* jid) } void -iq_last_activity_request(gchar* jid) +iq_last_activity_request(const char* jid) { xmpp_ctx_t* const ctx = connection_get_ctx(); auto_char char* id = connection_create_stanza_id(); @@ -618,7 +618,7 @@ iq_send_caps_request_legacy(const char* const to, const char* const id, } void -iq_disco_items_request(gchar* jid) +iq_disco_items_request(const char* jid) { xmpp_ctx_t* const ctx = connection_get_ctx(); xmpp_stanza_t* iq = stanza_create_disco_items_iq(ctx, "discoitemsreq", jid, NULL); @@ -627,7 +627,7 @@ iq_disco_items_request(gchar* jid) } void -iq_disco_items_request_onconnect(gchar* jid) +iq_disco_items_request_onconnect(const char* jid) { xmpp_ctx_t* const ctx = connection_get_ctx(); xmpp_stanza_t* iq = stanza_create_disco_items_iq(ctx, "discoitemsreq_onconnect", jid, NULL); @@ -1078,7 +1078,7 @@ _caps_response_legacy_id_handler(xmpp_stanza_t* const stanza, void* const userda static int _room_list_id_handler(xmpp_stanza_t* const stanza, void* const userdata) { - gchar* filter = (gchar*)userdata; + const char* filter = userdata; const char* id = xmpp_stanza_get_id(stanza); const char* from = xmpp_stanza_get_from(stanza); diff --git a/src/xmpp/iq.h b/src/xmpp/iq.h index 887e832e..9c36035e 100644 --- a/src/xmpp/iq.h +++ b/src/xmpp/iq.h @@ -42,8 +42,8 @@ typedef void (*ProfIqFreeCallback)(void* userdata); void iq_handlers_init(void); void iq_send_stanza(xmpp_stanza_t* const stanza); void iq_id_handler_add(const char* const id, ProfIqCallback func, ProfIqFreeCallback free_func, void* userdata); -void iq_disco_info_request_onconnect(gchar* jid); -void iq_disco_items_request_onconnect(gchar* jid); +void iq_disco_info_request_onconnect(const char* jid); +void iq_disco_items_request_onconnect(const char* jid); void iq_send_caps_request(const char* const to, const char* const id, const char* const node, const char* const ver); void iq_send_caps_request_for_jid(const char* const to, const char* const id, const char* const node, const char* const ver); diff --git a/src/xmpp/presence.c b/src/xmpp/presence.c index 2b0ae7f7..2a6b3157 100644 --- a/src/xmpp/presence.c +++ b/src/xmpp/presence.c @@ -192,7 +192,7 @@ presence_send(const resource_presence_t presence_type, const int idle, char* sig return; } - char* msg = connection_get_presence_msg(); + const char* msg = connection_get_presence_msg(); if (msg) { log_debug("Updating presence: %s, \"%s\"", string_from_resource_presence(presence_type), msg); } else { @@ -282,7 +282,7 @@ presence_join_room(const char* const room, const char* const nick, const char* c resource_presence_t presence_type = accounts_get_last_presence(session_get_account_name()); const char* show = stanza_get_presence_string_from_type(presence_type); - char* status = connection_get_presence_msg(); + const char* status = connection_get_presence_msg(); int pri = accounts_get_priority_for_presence_type(session_get_account_name(), presence_type); xmpp_ctx_t* ctx = connection_get_ctx(); @@ -306,7 +306,7 @@ presence_change_room_nick(const char* const room, const char* const nick) log_debug("Sending room nickname change to: %s, nick: %s", room, nick); resource_presence_t presence_type = accounts_get_last_presence(session_get_account_name()); const char* show = stanza_get_presence_string_from_type(presence_type); - char* status = connection_get_presence_msg(); + const char* status = connection_get_presence_msg(); int pri = accounts_get_priority_for_presence_type(session_get_account_name(), presence_type); auto_char char* full_room_jid = create_fulljid(room, nick); diff --git a/src/xmpp/session.c b/src/xmpp/session.c index 5e6ae09d..de6f626a 100644 --- a/src/xmpp/session.c +++ b/src/xmpp/session.c @@ -345,7 +345,7 @@ session_login_success(gboolean secured) // items discovery connection_request_features(); - char* domain = connection_get_domain(); + const char* domain = connection_get_domain(); iq_disco_items_request_onconnect(domain); if (prefs_get_boolean(PREF_CARBONS)) { diff --git a/src/xmpp/xmpp.h b/src/xmpp/xmpp.h index 57079d1f..3d92ae68 100644 --- a/src/xmpp/xmpp.h +++ b/src/xmpp/xmpp.h @@ -190,13 +190,13 @@ void session_reconnect_now(void); void connection_disconnect(void); jabber_conn_status_t connection_get_status(void); -char* connection_get_presence_msg(void); +const char* connection_get_presence_msg(void); void connection_set_presence_msg(const char* const message); const char* connection_get_fulljid(void); const Jid* connection_get_jid(void); const char* connection_get_barejid(void); gboolean equals_our_barejid(const char* cmp); -char* connection_get_user(void); +const char* connection_get_user(void); char* connection_create_uuid(void); void connection_free_uuid(char* uuid); TLSCertificate* connection_get_tls_peer_cert(void); @@ -205,7 +205,7 @@ gboolean connection_send_stanza(const char* const stanza); GList* connection_get_available_resources(void); int connection_count_available_resources(void); gboolean connection_supports(const char* const feature); -char* connection_jid_for_feature(const char* const feature); +const char* connection_jid_for_feature(const char* const feature); const char* connection_get_profanity_identifier(void); @@ -244,10 +244,10 @@ void iq_send_software_version(const char* const fulljid); void iq_rooms_cache_clear(void); void iq_handlers_remove_win(ProfWin* window); void iq_handlers_clear(void); -void iq_room_list_request(gchar* conferencejid, gchar* filter); -void iq_disco_info_request(gchar* jid); -void iq_disco_items_request(gchar* jid); -void iq_last_activity_request(gchar* jid); +void iq_room_list_request(const char* conferencejid, char* filter); +void iq_disco_info_request(const char* jid); +void iq_disco_items_request(const char* jid); +void iq_last_activity_request(const char* jid); void iq_set_autoping(int seconds); void iq_confirm_instant_room(const char* const room_jid); void iq_destroy_room(const char* const room_jid); diff --git a/tests/unittests/xmpp/stub_xmpp.c b/tests/unittests/xmpp/stub_xmpp.c index 4d6bd3df..f4e10a1e 100644 --- a/tests/unittests/xmpp/stub_xmpp.c +++ b/tests/unittests/xmpp/stub_xmpp.c @@ -69,10 +69,10 @@ connection_get_fulljid(void) return mock_ptr_type(char*); } -char* +const char* connection_get_barejid(void) { - return mock_ptr_type(char*); + return mock_ptr_type(const char*); } gboolean @@ -81,10 +81,10 @@ equals_our_barejid(const char* cmp) return TRUE; } -char* +const char* connection_get_user(void) { - return mock_ptr_type(char*); + return mock_ptr_type(const char*); } const char* @@ -122,10 +122,10 @@ connection_get_status(void) return mock_type(jabber_conn_status_t); } -char* +const char* connection_get_presence_msg(void) { - return mock_ptr_type(char*); + return mock_ptr_type(const char*); } char* @@ -324,18 +324,18 @@ iq_send_software_version(const char* const fulljid) } void -iq_room_list_request(gchar* conferencejid, gchar* filter) +iq_room_list_request(const char* conferencejid, char* filter) { check_expected(conferencejid); check_expected(filter); } void -iq_disco_info_request(gchar* jid) +iq_disco_info_request(const char* jid) { } void -iq_disco_items_request(gchar* jid) +iq_disco_items_request(const char* jid) { } void @@ -412,7 +412,7 @@ iq_room_role_list(const char* const room, char* role) { } void -iq_last_activity_request(gchar* jid) +iq_last_activity_request(const char* jid) { } void From 3c939264b3053f5e5d853d3e25cbf2eb907dcd03 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Sat, 9 Dec 2023 15:08:47 +0100 Subject: [PATCH 6/8] Less GString usage Signed-off-by: Steffen Jaeckel --- src/command/cmd_funcs.c | 109 +++++++++++++--------------------------- 1 file changed, 34 insertions(+), 75 deletions(-) diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index cd169169..f3dd5ba0 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -231,10 +231,8 @@ cmd_process_input(ProfWin* window, char* inp) void cmd_execute_connect(ProfWin* window, const char* const account) { - GString* command = g_string_new("/connect "); - g_string_append(command, account); - cmd_process_input(window, command->str); - g_string_free(command, TRUE); + auto_gchar gchar* command = g_strdup_printf("/connect %s", account); + cmd_process_input(window, command); } gboolean @@ -2207,21 +2205,16 @@ cmd_msg(ProfWin* window, const char* const command, gchar** args) cons_show("Starting direct message with occupant \"%s\" from room \"%s\" as \"%s\".", usr, mucwin->roomjid, jidp->barejid); } else { // otherwise send mucpm - GString* full_jid = g_string_new(mucwin->roomjid); - g_string_append(full_jid, "/"); - g_string_append(full_jid, usr); - - ProfPrivateWin* privwin = wins_get_private(full_jid->str); + auto_gchar gchar* full_jid = g_strdup_printf("%s/%s", mucwin->roomjid, usr); + ProfPrivateWin* privwin = wins_get_private(full_jid); if (!privwin) { - privwin = (ProfPrivateWin*)wins_new_private(full_jid->str); + privwin = (ProfPrivateWin*)wins_new_private(full_jid); } ui_focus_win((ProfWin*)privwin); if (msg) { cl_ev_send_priv_msg(privwin, msg, NULL); } - - g_string_free(full_jid, TRUE); } } else { @@ -3502,10 +3495,8 @@ cmd_software(ProfWin* window, const char* const command, gchar** args) } if (resource) { - GString* fulljid = g_string_new(chatwin->barejid); - g_string_append_printf(fulljid, "/%s", resource); - iq_send_software_version(fulljid->str); - g_string_free(fulljid, TRUE); + auto_gchar gchar* fulljid = g_strdup_printf("%s/%s", chatwin->barejid, resource); + iq_send_software_version(fulljid); } else { win_println(window, THEME_DEFAULT, "-", "Unknown resource for /software command. See /help resource."); } @@ -3565,20 +3556,17 @@ cmd_join(ProfWin* window, const char* const command, gchar** args) if (args[0] == NULL) { char* account_name = session_get_account_name(); ProfAccount* account = accounts_get_account(account_name); - if (account->muc_service) { - GString* room_str = g_string_new(""); + if (account && account->muc_service) { char* uuid = connection_create_uuid(); - g_string_append_printf(room_str, "private-chat-%s@%s", uuid, account->muc_service); + auto_gchar gchar* room_str = g_strdup_printf("private-chat-%s@%s", uuid, account->muc_service); connection_free_uuid(uuid); - presence_join_room(room_str->str, account->muc_nick, NULL); - muc_join(room_str->str, account->muc_nick, NULL, FALSE); - - g_string_free(room_str, TRUE); - account_free(account); + presence_join_room(room_str, account->muc_nick, NULL); + muc_join(room_str, account->muc_nick, NULL, FALSE); } else { cons_show("Account MUC service property not found."); } + account_free(account); return TRUE; } @@ -4122,10 +4110,8 @@ cmd_subject(ProfWin* window, const char* const command, gchar** args) if (args[1]) { char* old_subject = muc_subject(mucwin->roomjid); if (old_subject) { - GString* new_subject = g_string_new(args[1]); - g_string_append(new_subject, old_subject); - message_send_groupchat_subject(mucwin->roomjid, new_subject->str); - g_string_free(new_subject, TRUE); + auto_gchar gchar* new_subject = g_strdup_printf("%s%s", args[1], old_subject); + message_send_groupchat_subject(mucwin->roomjid, new_subject); } else { win_print(window, THEME_ROOMINFO, "!", "Room does not have a subject, use /subject set "); } @@ -4139,10 +4125,8 @@ cmd_subject(ProfWin* window, const char* const command, gchar** args) if (args[1]) { char* old_subject = muc_subject(mucwin->roomjid); if (old_subject) { - GString* new_subject = g_string_new(old_subject); - g_string_append(new_subject, args[1]); - message_send_groupchat_subject(mucwin->roomjid, new_subject->str); - g_string_free(new_subject, TRUE); + auto_gchar gchar* new_subject = g_strdup_printf("%s%s", old_subject, args[1]); + message_send_groupchat_subject(mucwin->roomjid, new_subject); } else { win_print(window, THEME_ROOMINFO, "!", "Room does not have a subject, use /subject set "); } @@ -4837,21 +4821,14 @@ cmd_disco(ProfWin* window, const char* const command, gchar** args) return TRUE; } - GString* jid = g_string_new(""); - if (args[1]) { - jid = g_string_append(jid, args[1]); - } else { - jid = g_string_append(jid, connection_get_jid()->domainpart); - } + auto_gchar gchar* jid = g_strdup_printf("%s", args[1] ?: connection_get_jid()->domainpart); if (g_strcmp0(args[0], "info") == 0) { - iq_disco_info_request(jid->str); + iq_disco_info_request(jid); } else { - iq_disco_items_request(jid->str); + iq_disco_items_request(jid); } - g_string_free(jid, TRUE); - return TRUE; } @@ -5086,32 +5063,26 @@ cmd_alias(ProfWin* window, const char* const command, gchar** args) return TRUE; } char* alias_p = alias; - GString* ac_value = g_string_new(""); + auto_gchar gchar* ac_value = NULL; if (alias[0] == '/') { - g_string_append(ac_value, alias); + ac_value = g_strdup_printf("%s", alias); alias_p = &alias[1]; } else { - g_string_append(ac_value, "/"); - g_string_append(ac_value, alias); + ac_value = g_strdup_printf("/%s", alias); } char* value = args[2]; if (value == NULL) { cons_bad_cmd_usage(command); - g_string_free(ac_value, TRUE); - return TRUE; - } else if (cmd_ac_exists(ac_value->str)) { - cons_show("Command or alias '%s' already exists.", ac_value->str); - g_string_free(ac_value, TRUE); - return TRUE; + } else if (cmd_ac_exists(ac_value)) { + cons_show("Command or alias '%s' already exists.", ac_value); } else { prefs_add_alias(alias_p, value); - cmd_ac_add(ac_value->str); + cmd_ac_add(ac_value); cmd_ac_add_alias_value(alias_p); - cons_show("Command alias added %s -> %s", ac_value->str, value); - g_string_free(ac_value, TRUE); - return TRUE; + cons_show("Command alias added %s -> %s", ac_value, value); } + return TRUE; } } else if (strcmp(subcmd, "remove") == 0) { char* alias = args[1]; @@ -5126,11 +5097,9 @@ cmd_alias(ProfWin* window, const char* const command, gchar** args) if (!removed) { cons_show("No such command alias /%s", alias); } else { - GString* ac_value = g_string_new("/"); - g_string_append(ac_value, alias); - cmd_ac_remove(ac_value->str); + auto_gchar gchar* ac_value = g_strdup_printf("/%s", alias); + cmd_ac_remove(ac_value); cmd_ac_remove_alias_value(alias); - g_string_free(ac_value, TRUE); cons_show("Command alias removed -> /%s", alias); } return TRUE; @@ -7429,10 +7398,7 @@ cmd_pgp(ProfWin* window, const char* const command, gchar** args) return TRUE; } - GString* fullstr = g_string_new("Using libgpgme version "); - g_string_append(fullstr, libver); - cons_show("%s", fullstr->str); - g_string_free(fullstr, TRUE); + cons_show("Using libgpgme version %s", libver); return TRUE; } @@ -7675,10 +7641,8 @@ cmd_ox(ProfWin* window, const char* const command, gchar** args) GSList* curr_c = roster_list; while (!contact && curr_c) { contact = curr_c->data; - const char* jid = p_contact_barejid(contact); - GString* xmppuri = g_string_new("xmpp:"); - g_string_append(xmppuri, jid); - if (g_strcmp0(key->name, xmppuri->str)) { + auto_gchar gchar* xmppuri = g_strdup_printf("xmpp:%s", p_contact_barejid(contact)); + if (g_strcmp0(key->name, xmppuri)) { contact = NULL; } curr_c = g_slist_next(curr_c); @@ -9999,13 +9963,8 @@ cmd_vcard_get(ProfWin* window, const char* const command, gchar** args) vcard_print(ctx, window, jid_occupant->barejid); } else { // anon muc: send the vcard request through the MUC's server - GString* full_jid = g_string_new(mucwin->roomjid); - g_string_append(full_jid, "/"); - g_string_append(full_jid, user); - - vcard_print(ctx, window, full_jid->str); - - g_string_free(full_jid, TRUE); + auto_gchar gchar* full_jid = g_strdup_printf("%s/%s", mucwin->roomjid, user); + vcard_print(ctx, window, full_jid); } } else { char* jid = roster_barejid_from_name(user); From dc0f2acb9a0202223b63c7f6de9cf302db7b171c Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Sat, 9 Dec 2023 15:08:47 +0100 Subject: [PATCH 7/8] Simplify usage of `roster_get_display_name()` Signed-off-by: Steffen Jaeckel --- src/plugins/api.c | 2 +- src/plugins/python_api.c | 2 +- src/ui/chatwin.c | 14 +------------- src/ui/console.c | 27 ++------------------------- src/ui/core.c | 13 +------------ src/xmpp/contact.c | 2 ++ src/xmpp/roster_list.c | 36 +++++------------------------------- src/xmpp/roster_list.h | 2 +- 8 files changed, 14 insertions(+), 84 deletions(-) diff --git a/src/plugins/api.c b/src/plugins/api.c index 31db05e3..032ae86a 100644 --- a/src/plugins/api.c +++ b/src/plugins/api.c @@ -240,7 +240,7 @@ api_get_current_nick(void) char* api_get_name_from_roster(const char* barejid) { - return roster_get_display_name(barejid); + return strdup(roster_get_display_name(barejid)); } char* diff --git a/src/plugins/python_api.c b/src/plugins/python_api.c index 091b3b4a..35a97d15 100644 --- a/src/plugins/python_api.c +++ b/src/plugins/python_api.c @@ -452,7 +452,7 @@ python_api_get_name_from_roster(PyObject* self, PyObject* args) char* barejid_str = python_str_or_unicode_to_string(barejid); allow_python_threads(); - char* name = roster_get_display_name(barejid_str); + char* name = strdup(roster_get_display_name(barejid_str)); free(barejid_str); disable_python_threads(); if (name) { diff --git a/src/ui/chatwin.c b/src/ui/chatwin.c index cf0b675f..4afa8c95 100644 --- a/src/ui/chatwin.c +++ b/src/ui/chatwin.c @@ -295,19 +295,7 @@ chatwin_recipient_gone(ProfChatWin* chatwin) { assert(chatwin != NULL); - const char* display_usr = NULL; - PContact contact = roster_get_contact(chatwin->barejid); - if (contact) { - if (p_contact_name(contact)) { - display_usr = p_contact_name(contact); - } else { - display_usr = chatwin->barejid; - } - } else { - display_usr = chatwin->barejid; - } - - win_println((ProfWin*)chatwin, THEME_GONE, "!", "<- %s has left the conversation.", display_usr); + win_println((ProfWin*)chatwin, THEME_GONE, "!", "<- %s has left the conversation.", roster_get_display_name(chatwin->barejid)); } void diff --git a/src/ui/console.c b/src/ui/console.c index dfbbfc14..c46faed5 100644 --- a/src/ui/console.c +++ b/src/ui/console.c @@ -267,20 +267,7 @@ cons_show_tlscert(const TLSCertificate* cert) void cons_show_typing(const char* const barejid) { - ProfWin* console = wins_get_console(); - const char* display_usr = NULL; - PContact contact = roster_get_contact(barejid); - if (contact) { - if (p_contact_name(contact)) { - display_usr = p_contact_name(contact); - } else { - display_usr = barejid; - } - } else { - display_usr = barejid; - } - - win_println(console, THEME_TYPING, "-", "!! %s is typing a messageā€¦", display_usr); + win_println(wins_get_console(), THEME_TYPING, "-", "!! %s is typing a messageā€¦", roster_get_display_name(barejid)); cons_alert(NULL); } @@ -925,17 +912,7 @@ cons_show_status(const char* const barejid) void cons_show_room_invite(const char* const invitor, const char* const room, const char* const reason) { - auto_char char* display_from = NULL; - PContact contact = roster_get_contact(invitor); - if (contact) { - if (p_contact_name(contact)) { - display_from = strdup(p_contact_name(contact)); - } else { - display_from = strdup(invitor); - } - } else { - display_from = strdup(invitor); - } + const char* display_from = roster_get_display_name(invitor); cons_show(""); cons_show("Chat room invite received:"); diff --git a/src/ui/core.c b/src/ui/core.c index 39535eaa..8c85070c 100644 --- a/src/ui/core.c +++ b/src/ui/core.c @@ -299,18 +299,7 @@ ui_contact_typing(const char* const barejid, const char* const resource) is_current = wins_is_current(window); } if (!is_current || (is_current && prefs_get_boolean(PREF_NOTIFY_TYPING_CURRENT))) { - PContact contact = roster_get_contact(barejid); - char const* display_usr = NULL; - if (contact) { - if (p_contact_name(contact)) { - display_usr = p_contact_name(contact); - } else { - display_usr = barejid; - } - } else { - display_usr = barejid; - } - notify_typing(display_usr); + notify_typing(roster_get_display_name(barejid)); } } } diff --git a/src/xmpp/contact.c b/src/xmpp/contact.c index 36ad3ce9..fd2889b9 100644 --- a/src/xmpp/contact.c +++ b/src/xmpp/contact.c @@ -192,6 +192,8 @@ p_contact_barejid_collate_key(const PContact contact) const char* p_contact_name(const PContact contact) { + if (!contact) + return NULL; return contact->name; } diff --git a/src/xmpp/roster_list.c b/src/xmpp/roster_list.c index b5a60db7..40b8210d 100644 --- a/src/xmpp/roster_list.c +++ b/src/xmpp/roster_list.c @@ -183,25 +183,12 @@ roster_get_contact(const char* const barejid) return contact; } -char* +const char* roster_get_display_name(const char* const barejid) { assert(roster != NULL); - GString* result = g_string_new(""); - - PContact contact = roster_get_contact(barejid); - if (contact) { - if (p_contact_name(contact)) { - g_string_append(result, p_contact_name(contact)); - } else { - g_string_append(result, barejid); - } - } else { - g_string_append(result, barejid); - } - - return g_string_free(result, FALSE); + return p_contact_name(roster_get_contact(barejid)) ?: barejid; } gchar* @@ -215,25 +202,12 @@ roster_get_msg_display_name(const char* const barejid, const char* const resourc return incoming_str; } - GString* result = g_string_new(""); - - PContact contact = roster_get_contact(barejid); - if (contact) { - if (p_contact_name(contact)) { - g_string_append(result, p_contact_name(contact)); - } else { - g_string_append(result, barejid); - } - } else { - g_string_append(result, barejid); - } + const char* contact_name = roster_get_display_name(barejid); if (resource && prefs_get_boolean(PREF_RESOURCE_MESSAGE)) { - g_string_append(result, "/"); - g_string_append(result, resource); + return g_strdup_printf("%s/%s", contact_name, resource); } - - return g_string_free(result, FALSE); + return g_strdup(contact_name); } gboolean diff --git a/src/xmpp/roster_list.h b/src/xmpp/roster_list.h index c31fe201..be29bd69 100644 --- a/src/xmpp/roster_list.h +++ b/src/xmpp/roster_list.h @@ -70,7 +70,7 @@ GList* roster_get_groups(void); char* roster_group_autocomplete(const char* const search_str, gboolean previous, void* context); char* roster_barejid_autocomplete(const char* const search_str, gboolean previous, void* context); GSList* roster_get_contacts_by_presence(const char* const presence); -char* roster_get_display_name(const char* const barejid); +const char* roster_get_display_name(const char* const barejid); gchar* roster_get_msg_display_name(const char* const barejid, const char* const resource); gint roster_compare_name(PContact a, PContact b); gint roster_compare_presence(PContact a, PContact b); From 801c6002af8bd99f363222d5db462cb8972a6e67 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Sat, 9 Dec 2023 15:08:47 +0100 Subject: [PATCH 8/8] Update Valgrind suppressions Signed-off-by: Steffen Jaeckel --- prof.supp | 1327 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 1281 insertions(+), 46 deletions(-) diff --git a/prof.supp b/prof.supp index f894dc75..57c73589 100644 --- a/prof.supp +++ b/prof.supp @@ -16,6 +16,14 @@ ... } +{ + gtk_init_check + Memcheck:Leak + ... + fun:gtk_init_check + ... +} + # glib { @@ -26,7 +34,16 @@ fun:clone } -## glib 2.60.4 suppressions file: +# gcrypt initialization +{ + gcry_rngcsprng_randomize + Memcheck:Leak + fun:malloc + ... + fun:omemo_crypto_init + ... +} + # GLib Valgrind suppressions file # @@ -52,6 +69,7 @@ { gnutls-init-calloc Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:gtls_gnutls_init @@ -60,6 +78,7 @@ { gnutls-init-realloc Memcheck:Leak + match-leak-kinds:reachable fun:realloc ... fun:gtls_gnutls_init @@ -68,6 +87,7 @@ { g-tls-backend-gnutls-init Memcheck:Leak + match-leak-kinds:reachable fun:g_once_impl fun:g_tls_backend_gnutls_init } @@ -75,6 +95,7 @@ { p11-tokens-init Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:create_tokens_inlock @@ -85,6 +106,7 @@ { g-local-vfs-getpwnam Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:getpwnam @@ -94,50 +116,52 @@ { glib-init-malloc Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:g_quark_init - ... - fun:glib_init_ctor } { glib-init-calloc Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:g_quark_init - ... - fun:glib_init_ctor } { gobject-init-malloc Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... - fun:gobject_init_ctor + fun:gobject_init* } { gobject-init-realloc Memcheck:Leak + match-leak-kinds:reachable fun:realloc ... - fun:gobject_init_ctor + fun:gobject_init* } { gobject-init-calloc Memcheck:Leak + match-leak-kinds:possible,reachable fun:calloc ... - fun:gobject_init_ctor + fun:gobject_init* } { g-type-register-dynamic Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:g_type_register_dynamic @@ -146,6 +170,7 @@ { g-type-register-static Memcheck:Leak + match-leak-kinds:possible,reachable fun:malloc ... fun:g_type_register_static @@ -154,6 +179,7 @@ { g-type-register-static-realloc Memcheck:Leak + match-leak-kinds:possible,reachable fun:realloc ... fun:g_type_register_static @@ -162,14 +188,34 @@ { g-type-register-static-calloc Memcheck:Leak + match-leak-kinds:possible,reachable fun:calloc ... fun:g_type_register_static } +{ + g-type-register-fundamental + Memcheck:Leak + match-leak-kinds:possible,reachable + fun:malloc + ... + fun:g_type_register_fundamental +} + +{ + g-type-register-fundamental-calloc + Memcheck:Leak + match-leak-kinds:possible,reachable + fun:calloc + ... + fun:g_type_register_fundamental +} + { g-type-add-interface-dynamic Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:g_type_add_interface_dynamic @@ -178,14 +224,34 @@ { g-type-add-interface-static Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:g_type_add_interface_static } +{ + g-type-add-interface-static-realloc + Memcheck:Leak + match-leak-kinds:reachable + fun:realloc + ... + fun:g_type_add_interface_static +} + +{ + g-type-add-interface-static-calloc + Memcheck:Leak + match-leak-kinds:reachable + fun:calloc + ... + fun:g_type_add_interface_static +} + { g-test-rand-init Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:g_rand_new_with_seed_array @@ -195,20 +261,20 @@ } { - g-test-rand-init2 + g-rand-init2 Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:g_rand_new_with_seed_array ... fun:get_global_random - ... - fun:g_test_init } { g-quark-table-new Memcheck:Leak + match-leak-kinds:reachable fun:g_hash_table_new ... fun:quark_new @@ -217,6 +283,8 @@ { g-quark-table-resize Memcheck:Leak + match-leak-kinds:reachable + ... fun:g_hash_table_resize ... fun:quark_new @@ -225,6 +293,7 @@ { g-type-interface-init Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:type_iface_vtable_base_init_Wm @@ -233,24 +302,131 @@ { g-type-class-init-calloc Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... - fun:g_type_create_instance - ... fun:type_class_init_Wm } { g-type-class-init Memcheck:Leak + match-leak-kinds:reachable fun:g_type_create_instance ... fun:type_class_init_Wm } +{ + g-object-do-class-init-signals + Memcheck:Leak + match-leak-kinds:reachable + ... + fun:g_signal_new + ... + fun:type_class_init_Wm +} + +{ + g-type-prerequisites + Memcheck:Leak + match-leak-kinds:reachable + fun:realloc + ... + fun:type_iface_add_prerequisite_W +} + +{ + g-type-add-interface-check + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + ... + fun:g_type_add_interface_check + ... + fun:type_class_init_Wm +} + +{ + g-type-add-interface-check-realloc + Memcheck:Leak + match-leak-kinds:reachable + fun:realloc + ... + fun:g_type_add_interface_check + ... + fun:type_class_init_Wm +} + +{ + g-object-class-install-property + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + ... + fun:validate_and_install_class_property + ... + fun:type_class_init_Wm +} + +{ + g-param-spec-pool-new + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + ... + fun:g_param_spec_pool_new + ... + fun:type_class_init_Wm +} + +# weak_locations_lock in gobject.c +{ + g-weak-ref-lock + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + ... + fun:g_rw_lock_get_impl + ... + fun:g_weak_ref_set +} + +{ + g-object-base-class-init-construct-pproperties + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + ... + fun:g_slist_copy + fun:g_object_base_class_init + fun:type_class_init_Wm +} + +{ + g-type-class-ref + Memcheck:Leak + fun:calloc + ... + fun:type_class_init_Wm + ... + fun:g_type_class_ref +} + +{ + g-type-class-ref-inlined + Memcheck:Leak + fun:calloc + ... + fun:UnknownInlinedFun + ... + fun:g_type_class_ref +} + { g-io-module-default-singleton-malloc Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:g_type_create_instance @@ -261,6 +437,7 @@ { g-io-module-default-singleton-calloc Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:g_type_create_instance @@ -273,6 +450,7 @@ { g-io-module-default-singleton Memcheck:Leak + match-leak-kinds:reachable fun:g_type_create_instance ... fun:_g_io_module_get_default @@ -281,6 +459,7 @@ { g-io-module-default-singleton-module Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:g_module_open @@ -291,6 +470,7 @@ { g-io-module-default-singleton-name Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:g_strdup @@ -298,9 +478,18 @@ fun:_g_io_module_get_default* } +{ + g-io-module-default-singleton-weak-ref + Memcheck:Leak + fun:calloc + ... + fun:_g_io_module_get_default +} + { g-get-language-names-malloc Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:g_get_language_names @@ -309,14 +498,43 @@ { g-get-language-names-calloc Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:g_get_language_names } +{ + g-get-language_names-with-category-malloc + Memcheck:Leak + match-leak-kinds:possible,reachable,definite + fun:malloc + ... + fun:g_get_language_names_with_category +} + +{ + g-get-language_names-with-category-calloc + Memcheck:Leak + match-leak-kinds:possible,reachable,definite + fun:calloc + ... + fun:g_get_language_names_with_category +} + +{ + g-get-language_names-with-category-realloc + Memcheck:Leak + match-leak-kinds:possible,reachable,definite + fun:realloc + ... + fun:g_get_language_names_with_category +} + { g-static-mutex Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:g_static_mutex_get_mutex_impl @@ -325,14 +543,47 @@ { g-system-thread-init Memcheck:Leak + match-leak-kinds:possible,reachable fun:calloc ... fun:g_system_thread_new } +{ + g-system-thread-init-malloc + Memcheck:Leak + match-leak-kinds:possible,reachable,definite + fun:malloc + ... + fun:g_system_thread_new +} + +{ + g-task-thread-pool-init + Memcheck:Leak + match-leak-kinds:possible,reachable,definite + fun:malloc + ... + fun:g_thread_new + ... + fun:g_task_thread_pool_init +} + +{ + g-task-thread-pool-new-full + Memcheck:Leak + match-leak-kinds:possible,reachable,definite + fun:malloc + ... + fun:g_thread_new + ... + fun:g_thread_pool_new_full +} + { g-io-module-default-proxy-resolver-gnome Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:g_proxy_resolver_gnome_init @@ -344,6 +595,7 @@ { g-threaded-resolver-getaddrinfo-config Memcheck:Leak + match-leak-kinds:reachable,definite fun:malloc ... fun:__resolv_conf_allocate @@ -355,11 +607,11 @@ # memcheck checks that the third argument to ioctl() is a valid pointer, but # some ioctls use that argument as an integer { - ioctl-with-non-pointer-param - Memcheck:Param - ioctl(generic) - fun:ioctl - fun:btrfs_reflink_with_progress + ioctl-with-non-pointer-param + Memcheck:Param + ioctl(generic) + fun:ioctl + fun:btrfs_reflink_with_progress } { @@ -506,7 +758,7 @@ fun:g_inet_address_get_type } -# From: https://github.com/fredericgermain/valgrind/blob/master/glibc-2.X-drd.supp +# From: https://github.com/fredericgermain/valgrind/blob/HEAD/glibc-2.X-drd.supp { drd-libc-stdio drd:ConflictingAccess @@ -623,9 +875,11 @@ } # g_set_user_dirs() deliberately leaks the previous cached g_get_user_*() values. +# These will not all be reachable on exit. { g_set_user_dirs_str Memcheck:Leak + match-leak-kinds:definite,reachable fun:malloc ... fun:set_str_if_different @@ -633,19 +887,45 @@ } # g_set_user_dirs() deliberately leaks the previous cached g_get_user_*() values. +# These will not all be reachable on exit. { g_set_user_dirs_strv Memcheck:Leak + match-leak-kinds:definite,reachable fun:malloc ... fun:set_strv_if_different fun:g_set_user_dirs } +# _g_unset_cached_tmp_dir() deliberately leaks the previous cached g_get_tmp_dir() values. +# These will not all be reachable on exit. +{ + g_get_tmp_dir_test_init + Memcheck:Leak + match-leak-kinds:definite,reachable + fun:malloc + ... + fun:g_get_tmp_dir + ... + fun:g_test_init +} + +# g_get_tmp_dir() caches a one-time allocation +{ + g_get_tmp_dir + Memcheck:Leak + match-leak-kinds:definite,reachable + fun:malloc + ... + fun:g_get_tmp_dir +} + # g_get_system_data_dirs() caches a one-time allocation { g_get_system_data_dirs Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:g_build_system_data_dirs @@ -656,16 +936,29 @@ { g_get_user_data_dir Memcheck:Leak + match-leak-kinds:reachable fun:realloc ... fun:g_build_user_data_dir fun:g_get_user_data_dir } +# g_get_home_dir() caches a one-time allocation +{ + g_get_home_dir + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + ... + fun:g_build_home_dir + fun:g_get_home_dir +} + # gdesktopappinfo.c caches a one-time allocation global table of @desktop_file_dirs. { desktop_file_dirs_malloc Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:desktop_file_dirs_lock @@ -675,6 +968,7 @@ { desktop_file_dirs_realloc Memcheck:Leak + match-leak-kinds:reachable fun:realloc ... fun:desktop_file_dirs_lock @@ -684,16 +978,138 @@ { desktop_file_dir_unindexed_setup_search Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:desktop_file_dir_unindexed_setup_search fun:desktop_file_dir_unindexed_setup_search } +#gutils.c caches system and user dirs and may need to replace them during tests. +{ + g_build_user_data_dir + Memcheck:Leak + match-leak-kinds:definite + fun:malloc + ... + fun:g_build_user_data_dir +} + +#gutils.c caches system and user dirs and may need to replace them during tests. +{ + g_build_filename + Memcheck:Leak + match-leak-kinds:definite + fun:malloc + ... + fun:g_build_filename +} + +#gutils.c caches system and user dirs and may need to replace them during tests. +{ + g_build_home_dir + Memcheck:Leak + match-leak-kinds:definite + fun:malloc + ... + fun:g_build_home_dir +} + +#gutils.c caches system and user dirs and may need to replace them during tests. +{ + g_build_path + Memcheck:Leak + match-leak-kinds:definite + fun:malloc + ... + fun:g_build_path +} + +#gutils.c caches system and user dirs and may need to replace them during tests. +{ + g_build_system_config_dirs + Memcheck:Leak + match-leak-kinds:definite + fun:realloc + ... + fun:g_build_system_config_dirs +} + +#gutils.c caches system and user dirs and may need to replace them during tests. +{ + g_build_system_data_dir + Memcheck:Leak + match-leak-kinds:definite + fun:malloc + ... + fun:g_build_system_data_dir +} + +#gutils.c caches system and user dirs and may need to replace them during tests. +{ + g_build_system_data_dirs + Memcheck:Leak + match-leak-kinds:definite + fun:realloc + ... + fun:g_build_system_data_dirs +} + +#gutils.c caches system and user dirs and may need to replace them during tests. +{ + g_build_user_cache_dir + Memcheck:Leak + match-leak-kinds:definite + fun:malloc + ... + fun:g_build_user_cache_dir +} + +#gutils.c caches system and user dirs and may need to replace them during tests. +{ + g_build_user_config_dir + Memcheck:Leak + match-leak-kinds:definite + fun:malloc + ... + fun:g_build_user_config_dir +} + +#gutils.c caches system and user dirs and may need to replace them during tests. +{ + g_build_user_data_dir + Memcheck:Leak + match-leak-kinds:definite + fun:malloc + ... + fun:g_build_user_data_dir +} + +#gutils.c caches system and user dirs and may need to replace them during tests. +{ + g_build_user_runtime_dir + Memcheck:Leak + match-leak-kinds:definite + fun:malloc + ... + fun:g_build_user_runtime_dir +} + +#gutils.c caches system and user dirs and may need to replace them during tests. +{ + g_build_user_state_dir + Memcheck:Leak + match-leak-kinds:definite + fun:malloc + ... + fun:g_build_user_state_dir +} + # g_io_extension_point_register() caches a one-time allocation global table of @extension_points. { g_io_extension_point_register Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:g_io_extension_point_register @@ -703,6 +1119,7 @@ { g_strerror Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:g_locale_to_utf8 @@ -713,6 +1130,7 @@ { g_socket_connection_factory_register_type Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:g_socket_connection_factory_register_type @@ -722,25 +1140,55 @@ { g_dbus_error_quark Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:g_dbus_error_register_error_domain fun:g_dbus_error_quark } +# g_win32_registry_get_os_dirs_w*() caches an array of strings that is allocated only once. +{ + g_win32_registry_get_os_dirs + Memcheck:Leak + match-leak-kinds:reachable,definite + fun:malloc + ... + fun:g_win32_registry_get_os_dirs* +} + # Thread-private data allocated once per thread { g_private_set_alloc0 Memcheck:Leak + match-leak-kinds:definite,reachable fun:malloc ... fun:g_private_set_alloc0 } +{ + g_private_set_alloc0-calloc + Memcheck:Leak + match-leak-kinds:definite,reachable + fun:calloc + ... + fun:g_private_set_alloc0 +} + +# Keys for thread-private data +{ + g_private_key + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + fun:g_private_impl_new +} # Thread-private GMainContext stack { g_main_context_push_thread_default Memcheck:Leak + match-leak-kinds:definite,reachable fun:malloc ... fun:g_queue_new @@ -751,6 +1199,7 @@ { g_file_info_attribute_cache Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:ensure_attribute_hash @@ -760,6 +1209,7 @@ { g_file_info_attribute_cache2 Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:ensure_attribute_hash @@ -769,6 +1219,7 @@ { g_file_info_attribute_cache3 Memcheck:Leak + match-leak-kinds:reachable fun:malloc ... fun:lookup_namespace @@ -778,6 +1229,7 @@ { g_file_info_attribute_cache4 Memcheck:Leak + match-leak-kinds:reachable fun:calloc ... fun:lookup_namespace @@ -785,7 +1237,95 @@ fun:g_file_* } -## python suppressions file: +# Cached charset +{ + g_get_charset + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + ... + fun:g_get_charset +} + +{ + g_get_charset_calloc + Memcheck:Leak + match-leak-kinds:reachable + fun:calloc + ... + fun:g_get_charset +} + +# Global unused thread queue +{ + g_thread_pool_unused_thread_queue + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + ... + fun:g_async_queue_new_full + ... + fun:g_thread_pool_new +} + +# One-time program name storage +{ + g_set_prgname + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + ... + fun:g_set_prgname +} + +# Error domains hash +{ + g_error_init + Memcheck:Leak + match-leak-kinds: reachable + fun:malloc + ... + fun:g_hash_table_new_full + fun:g_error_init +} + +# Error domain static registration +{ + g_error_domain_register_static + Memcheck:Leak + match-leak-kinds: reachable + fun:malloc + ... + fun:g_hash_table_insert + fun:error_domain_register + fun:g_error_domain_register_static +} + +{ + new_quark + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + ... + fun:g_hash_table_insert + fun:quark_new +} + +{ + xdg_mime_init_malloc + Memcheck:Leak + fun:malloc + ... + fun:xdg_mime_init +} + +{ + xdg_mime_init_calloc + Memcheck:Leak + fun:calloc + ... + fun:xdg_mime_init +} # # This is a valgrind suppression file that should be used when using valgrind. # @@ -793,13 +1333,13 @@ # # cd python/dist/src # valgrind --tool=memcheck --suppressions=Misc/valgrind-python.supp \ -# ./python -E -tt ./Lib/test/regrtest.py -u bsddb,network +# ./python -E ./Lib/test/regrtest.py -u gui,network # # You must edit Objects/obmalloc.c and uncomment Py_USING_MEMORY_DEBUGGER -# to use the preferred suppressions with Py_ADDRESS_IN_RANGE. +# to use the preferred suppressions with address_in_range. # # If you do not want to recompile Python, you can uncomment -# suppressions for PyObject_Free and PyObject_Realloc. +# suppressions for _PyObject_Free and _PyObject_Realloc. # # See Misc/README.valgrind for more information. @@ -807,25 +1347,25 @@ { ADDRESS_IN_RANGE/Invalid read of size 4 Memcheck:Addr4 - fun:Py_ADDRESS_IN_RANGE + fun:address_in_range } { ADDRESS_IN_RANGE/Invalid read of size 4 Memcheck:Value4 - fun:Py_ADDRESS_IN_RANGE + fun:address_in_range } { ADDRESS_IN_RANGE/Invalid read of size 8 (x86_64 aka amd64) Memcheck:Value8 - fun:Py_ADDRESS_IN_RANGE + fun:address_in_range } { ADDRESS_IN_RANGE/Conditional jump or move depends on uninitialised value Memcheck:Cond - fun:Py_ADDRESS_IN_RANGE + fun:address_in_range } # @@ -834,14 +1374,6 @@ # Will need to fix that. # -{ - Suppress leaking the GIL. Happens once per process, see comment in ceval.c. - Memcheck:Leak - fun:malloc - fun:PyThread_allocate_lock - fun:PyEval_InitThreads -} - { Suppress leaking the GIL after a fork. Memcheck:Leak @@ -915,37 +1447,61 @@ ###{ ### ADDRESS_IN_RANGE/Invalid read of size 4 ### Memcheck:Addr4 -### fun:PyObject_Free +### fun:_PyObject_Free ###} ### ###{ ### ADDRESS_IN_RANGE/Invalid read of size 4 ### Memcheck:Value4 -### fun:PyObject_Free +### fun:_PyObject_Free +###} +### +###{ +### ADDRESS_IN_RANGE/Use of uninitialised value of size 8 +### Memcheck:Addr8 +### fun:_PyObject_Free +###} +### +###{ +### ADDRESS_IN_RANGE/Use of uninitialised value of size 8 +### Memcheck:Value8 +### fun:_PyObject_Free ###} ### ###{ ### ADDRESS_IN_RANGE/Conditional jump or move depends on uninitialised value ### Memcheck:Cond -### fun:PyObject_Free +### fun:_PyObject_Free ###} ###{ ### ADDRESS_IN_RANGE/Invalid read of size 4 ### Memcheck:Addr4 -### fun:PyObject_Realloc +### fun:_PyObject_Realloc ###} ### ###{ ### ADDRESS_IN_RANGE/Invalid read of size 4 ### Memcheck:Value4 -### fun:PyObject_Realloc +### fun:_PyObject_Realloc +###} +### +###{ +### ADDRESS_IN_RANGE/Use of uninitialised value of size 8 +### Memcheck:Addr8 +### fun:_PyObject_Realloc +###} +### +###{ +### ADDRESS_IN_RANGE/Use of uninitialised value of size 8 +### Memcheck:Value8 +### fun:_PyObject_Realloc ###} ### ###{ ### ADDRESS_IN_RANGE/Conditional jump or move depends on uninitialised value ### Memcheck:Cond -### fun:PyObject_Realloc +### fun:_PyObject_Realloc ###} ### @@ -1027,6 +1583,14 @@ } +{ + Uninitialised byte(s) false alarm, see bpo-35561 + Memcheck:Param + epoll_ctl(event) + fun:epoll_ctl + fun:pyepoll_internal_ctl +} + { ZLIB problems, see test_gzip Memcheck:Cond @@ -1047,6 +1611,17 @@ fun:rl_initialize } +# Valgrind emits "Conditional jump or move depends on uninitialised value(s)" +# false alarms on GCC builtin strcmp() function. The GCC code is correct. +# +# Valgrind bug: https://bugs.kde.org/show_bug.cgi?id=264936 +{ + bpo-38118: Valgrind emits false alarm on GCC builtin strcmp() + Memcheck:Cond + fun:PyUnicode_Decode +} + + ### ### These occur from somewhere within the SSL, when running ### test_socket_sll. They are too general to leave on by default. @@ -1074,6 +1649,38 @@ ### fun:MD5_Update ###} +# Fedora's package "openssl-1.0.1-0.1.beta2.fc17.x86_64" on x86_64 +# See http://bugs.python.org/issue14171 +{ + openssl 1.0.1 prng 1 + Memcheck:Cond + fun:bcmp + fun:fips_get_entropy + fun:FIPS_drbg_instantiate + fun:RAND_init_fips + fun:OPENSSL_init_library + fun:SSL_library_init + fun:init_hashlib +} + +{ + openssl 1.0.1 prng 2 + Memcheck:Cond + fun:fips_get_entropy + fun:FIPS_drbg_instantiate + fun:RAND_init_fips + fun:OPENSSL_init_library + fun:SSL_library_init + fun:init_hashlib +} + +{ + openssl 1.0.1 prng 3 + Memcheck:Value8 + fun:_x86_64_AES_encrypt_compact + fun:AES_encrypt +} + # # All of these problems come from using test_socket_ssl # @@ -1176,13 +1783,641 @@ fun:SHA1_Update } - -# gcrypt initialization { - gcry_rngcsprng_randomize + test_buffer_non_debug + Memcheck:Addr4 + fun:PyUnicodeUCS2_FSConverter +} + +{ + test_buffer_non_debug + Memcheck:Addr4 + fun:PyUnicode_FSConverter +} + +{ + wcscmp_false_positive + Memcheck:Addr8 + fun:wcscmp + fun:_PyOS_GetOpt + fun:Py_Main + fun:main +} + +# Additional suppressions for the unified decimal tests: +{ + test_decimal + Memcheck:Addr4 + fun:PyUnicodeUCS2_FSConverter +} + +{ + test_decimal2 + Memcheck:Addr4 + fun:PyUnicode_FSConverter +} + +# Actual GTK things +{ + GtkWidgetClass action GPtrArray + Memcheck:Leak + fun:malloc + fun:g_malloc + fun:g_slice_alloc + fun:g_ptr_array_sized_new + fun:g_ptr_array_new + fun:gtk_widget_class_add_action +} + +{ + GTK media extension gio modules + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:g_malloc + fun:g_slice_alloc + fun:g_slice_alloc0 + fun:g_type_create_instance + fun:g_object_new_internal + fun:g_object_new_with_properties + fun:g_object_new + fun:g_io_module_new + fun:g_io_modules_scan_all_in_directory_with_scope + fun:gtk_media_file_extension_init +} + +{ + gtk-style-context + Memcheck:Leak + match-leak-kinds: possible + fun:malloc + fun:g_malloc + ... + fun:gtk_css_node_declaration_make_writable + ... + fun:gtk_style_constructed +} + +{ + gtk-style-context2 + Memcheck:Leak + match-leak-kinds: possible + fun:malloc + fun:g_malloc + ... + fun:gtk_css_node_declaration_make_writable_resize + ... + fun:gtk_style_constructed +} + +# AMD driver +{ + radeonsi_dri general + Memcheck:Leak + fun:calloc + ... + obj:/usr/lib*/dri/radeonsi_dri.so +} + +# mesa driver stuff +{ + i965 addr4 + Memcheck:Addr4 + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + i965 addr8 + Memcheck:Addr8 + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + i965 memcpy + Memcheck:Addr8 + fun:memcpy* + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + i965 memcpy + Memcheck:Addr2 + fun:memcpy* + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + mesa memcmp 8 + Memcheck:Addr8 + fun:*memcmp* + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + mesa memcmp 1 + Memcheck:Addr1 + fun:*memcmp* + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + mesa memset 8 + Memcheck:Addr8 + fun:*memset* + obj:/usr/lib*/dri/i965_dri.so +} + +{ + mesa realpath + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:realpath@@GLIBC_2.3 + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + fun:epoxy_eglInitialize_global_rewrite_ptr +} + +{ + mesa calloc + Memcheck:Leak + match-leak-kinds: definite + fun:calloc + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + fun:epoxy_eglInitialize_global_rewrite_ptr +} + +{ + mesa malloc + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + mesa glReadPixels + Memcheck:Addr16 + obj:* + obj:* + obj:* + obj:* + obj:* + fun:epoxy_glReadPixels_global_rewrite_ptr +} + +{ + epoxy glxQueryServerString 1 + Memcheck:Leak + fun:malloc + fun:XextAddDisplay + obj:* + obj:* + obj:* + obj:* + obj:* + fun:epoxy_glXQueryServerString_global_rewrite_ptr + +} + +{ + epoxy glxQueryServerString 2 + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:realpath* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + fun:epoxy_glXQueryServerString_global_rewrite_ptr +} + +{ + epoxy glGetTexImage + Memcheck:Addr16 + obj:* + obj:* + obj:* + obj:* + obj:* + fun:epoxy_glGetTexImage_global_rewrite_ptr +} + + + + +# Fontconfig +{ + FcFontSetList + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + obj:/usr/lib*/libfontconfig.so* + obj:/usr/lib*/libfontconfig.so* + fun:FcFontSetList +} + +{ + FcFontRenderPrepare + Memcheck:Leak + match-leak-kinds: definite + fun:realloc + obj:/usr/lib*/libfontconfig.so* + obj:/usr/lib*/libfontconfig.so* + fun:FcFontRenderPrepare +} + +{ + FcDefaultSubstitute + Memcheck:Leak + match-leak-kinds: definite + fun:realloc + obj:/usr/lib*/libfontconfig.so* + obj:/usr/lib*/libfontconfig.so* + fun:FcDefaultSubstitute +} + +# Pixman +{ + pixman_image_composite32 + Memcheck:Cond + obj:/usr/lib*/libpixman-1.so* + obj:/usr/lib*/libpixman-1.so* + fun:pixman_image_composite32 +} + +# Pango +{ + pango 1 + Memcheck:Leak + match-leak-kinds: definite + fun:realloc + obj:/usr/lib*/libfontconfig.so* + obj:/usr/lib*/libfontconfig.so* + obj:/usr/lib*/libcairo.so* + fun:pango_cairo_fc_font_map_fontset_key_substitute +} + +{ + pango 2 + Memcheck:Leak + fun:realloc + obj:/usr/lib*/libfontconfig.so* + obj:/usr/lib*/libfontconfig.so* + fun:_cairo_ft_font_options_substitute +} + +# GLib +{ + glib 1 + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:g_malloc + fun:g_quark_init +} +# Actual GTK things +{ + GtkWidgetClass action GPtrArray + Memcheck:Leak + fun:malloc + fun:g_malloc + fun:g_slice_alloc + fun:g_ptr_array_sized_new + fun:g_ptr_array_new + fun:gtk_widget_class_add_action +} + +{ + GIO modules + Memcheck:Leak + match-leak-kinds: definite + fun:calloc + ... + fun:_g_io_module_get_default +} + +{ + GTK media extension gio modules + Memcheck:Leak + match-leak-kinds: definite + fun:calloc + ... + fun:g_io_module_new + ... + fun:gtk_media_file_extension_init +} + +# AMD driver +{ + radeonsi_dri general + Memcheck:Leak + fun:calloc + ... + obj:/usr/lib*/dri/radeonsi_dri.so +} +{ + radeonsi_dri general Memcheck:Leak fun:malloc ... - fun:omemo_crypto_init - ... + obj:/usr/lib*/dri/radeonsi_dri.so +} + +# mesa driver stuff +{ + i965 addr4 + Memcheck:Addr4 + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + i965 addr8 + Memcheck:Addr8 + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + i965 memcpy + Memcheck:Addr8 + fun:memcpy* + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + i965 memcpy + Memcheck:Addr2 + fun:memcpy* + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + mesa memcmp 8 + Memcheck:Addr8 + fun:*memcmp* + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + mesa memcmp 1 + Memcheck:Addr1 + fun:*memcmp* + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + mesa memset 8 + Memcheck:Addr8 + fun:*memset* + obj:/usr/lib*/dri/i965_dri.so +} + +{ + mesa realpath + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:realpath@@GLIBC_2.3 + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + fun:epoxy_eglInitialize_global_rewrite_ptr +} + +{ + mesa calloc + Memcheck:Leak + match-leak-kinds: definite + fun:calloc + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + fun:epoxy_eglInitialize_global_rewrite_ptr +} + +{ + epoxy strncmp + Memcheck:Addr8 + fun:strncmp + ... + fun:epoxy_eglInitialize_global_rewrite_ptr +} + +{ + mesa malloc + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + obj:/usr/lib*/dri/i965_dri.so* +} + +{ + mesa glReadPixels + Memcheck:Addr16 + obj:* + obj:* + obj:* + obj:* + obj:* + fun:epoxy_glReadPixels_global_rewrite_ptr +} + +{ + epoxy glxQueryServerString 1 + Memcheck:Leak + fun:malloc + fun:XextAddDisplay + obj:* + obj:* + obj:* + obj:* + obj:* + fun:epoxy_glXQueryServerString_global_rewrite_ptr + +} + +{ + epoxy glxQueryServerString 2 + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:realpath* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + fun:epoxy_glXQueryServerString_global_rewrite_ptr +} + +{ + epoxy glGetTexImage + Memcheck:Addr16 + obj:* + obj:* + obj:* + obj:* + obj:* + fun:epoxy_glGetTexImage_global_rewrite_ptr +} + + + + +# Fontconfig +{ + FcFontSetList + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + obj:/usr/lib*/libfontconfig.so* + obj:/usr/lib*/libfontconfig.so* + fun:FcFontSetList +} + +{ + FcPatternObjectInsertElt + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:FcPatternObjectInsertElt +} + +{ + FcFontRenderPrepare + Memcheck:Leak + match-leak-kinds: definite + fun:realloc + obj:/usr/lib*/libfontconfig.so* + obj:/usr/lib*/libfontconfig.so* + fun:FcFontRenderPrepare +} + +{ + FcDefaultSubstitute + Memcheck:Leak + match-leak-kinds: definite + fun:realloc + obj:/usr/lib*/libfontconfig.so* + obj:/usr/lib*/libfontconfig.so* + fun:FcDefaultSubstitute +} + +# Pixman +{ + pixman_image_composite32 + Memcheck:Cond + obj:/usr/lib*/libpixman-1.so* + obj:/usr/lib*/libpixman-1.so* + fun:pixman_image_composite32 +} + +# Pango +{ + pango 1 + Memcheck:Leak + match-leak-kinds: definite + fun:realloc + obj:/usr/lib*/libfontconfig.so* + obj:/usr/lib*/libfontconfig.so* + obj:/usr/lib*/libcairo.so* + fun:pango_cairo_fc_font_map_fontset_key_substitute +} + +{ + pango 2 + Memcheck:Leak + fun:realloc + obj:/usr/lib*/libfontconfig.so* + obj:/usr/lib*/libfontconfig.so* + fun:_cairo_ft_font_options_substitute +} + +# GLib +{ + glib GQuark + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + ... + fun:g_quark_* +} +{ + glib GQuark + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + ... + fun:g_intern_static_string +} +{ + glib GQuark + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + ... + fun:g_intern_string +} +{ + xdg-mime init + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + ... + fun:xdg_mime_init* +} +{ + xdg-mime init + Memcheck:Leak + match-leak-kinds: definite + fun:calloc + ... + fun:xdg_mime_init* +} +{ + glib init + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + ... + fun:glib_init_ctor +} + +# Threads +{ + pthread + Memcheck:Leak + fun:calloc + fun:_dl_allocate_tls }