Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some fixes #1941

Merged
merged 6 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/plugins/plugins.c
Original file line number Diff line number Diff line change
Expand Up @@ -932,21 +932,23 @@ void
plugins_shutdown(void)
{
GList* values = g_hash_table_get_values(plugins);
GList* curr = values;
GList *curr = values, *next;

while (curr) {
next = g_list_next(curr);
#ifdef HAVE_PYTHON
if (((ProfPlugin*)curr->data)->lang == LANG_PYTHON) {
if (curr && ((ProfPlugin*)curr->data)->lang == LANG_PYTHON) {
python_plugin_destroy(curr->data);
curr = NULL;
}
#endif
#ifdef HAVE_C
if (((ProfPlugin*)curr->data)->lang == LANG_C) {
if (curr && ((ProfPlugin*)curr->data)->lang == LANG_C) {
c_plugin_destroy(curr->data);
curr = NULL;
}
#endif

curr = g_list_next(curr);
curr = next;
}
g_list_free(values);
#ifdef HAVE_PYTHON
Expand Down
2 changes: 1 addition & 1 deletion src/profanity.c
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,9 @@ _shutdown(void)
accounts_close();
tlscerts_close();
log_stderr_close();
log_close();
plugins_shutdown();
cmd_uninit();
ui_close();
prefs_close();
log_close();
}
2 changes: 2 additions & 0 deletions src/ui/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,8 @@ ui_close_win(int index)
}
}

// remove the IQ handlers
iq_handlers_remove_win(window);
wins_close_by_num(index);
title_bar_console();
status_bar_current(1);
Expand Down
16 changes: 9 additions & 7 deletions src/xmpp/connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ _conn_apply_settings(const char* const jid, const char* const passwd, const char

_compute_identifier(jidp->barejid);

xmpp_ctx_set_verbosity(conn.xmpp_ctx, 0);
xmpp_conn_set_jid(conn.xmpp_conn, jid);
if (passwd)
xmpp_conn_set_pass(conn.xmpp_conn, passwd);
Expand Down Expand Up @@ -561,7 +560,7 @@ connection_disconnect(void)
}
}

free(prof_identifier);
g_free(prof_identifier);
prof_identifier = NULL;
}

Expand Down Expand Up @@ -998,22 +997,25 @@ _connection_handler(xmpp_conn_t* const xmpp_conn, const xmpp_conn_event_t status
log_debug("Connection handler: XMPP_CONN_DISCONNECT");

// lost connection for unknown reason
if (conn.conn_status == JABBER_CONNECTED) {
if (conn.conn_status == JABBER_CONNECTED || conn.conn_status == JABBER_DISCONNECTING) {
if (prefs_get_boolean(PREF_STROPHE_SM_ENABLED)) {
int send_queue_len = xmpp_conn_send_queue_len(conn.xmpp_conn);
log_debug("Connection handler: Lost connection for unknown reason");
log_debug("Connection handler: Lost connection for unknown reason, %d messages in send queue", send_queue_len);
conn.sm_state = xmpp_conn_get_sm_state(conn.xmpp_conn);
if (send_queue_len > 0 && prefs_get_boolean(PREF_STROPHE_SM_RESEND)) {
conn.queued_messages = calloc(send_queue_len + 1, sizeof(*conn.queued_messages));
for (int n = 0; n < send_queue_len && conn.queued_messages[n]; ++n) {
conn.queued_messages[n] = xmpp_conn_send_queue_drop_element(conn.xmpp_conn, XMPP_QUEUE_OLDEST);
}
} else if (send_queue_len > 0) {
log_debug("Connection handler: dropping those messages since SM RESEND is disabled");
}
}
session_lost_connection();
if (conn.conn_status == JABBER_CONNECTED)
session_lost_connection();

// login attempt failed
} else if (conn.conn_status != JABBER_DISCONNECTING) {
} else {
gchar* host;
int port;
if (stream_error && stream_error->stanza && _get_other_host(stream_error->stanza, &host, &port)) {
Expand Down Expand Up @@ -1126,7 +1128,7 @@ static void
_compute_identifier(const char* barejid)
{
// in case of reconnect (lost connection)
free(prof_identifier);
g_free(prof_identifier);

prof_identifier = g_compute_hmac_for_string(G_CHECKSUM_SHA256,
(guchar*)profanity_instance_id, strlen(profanity_instance_id),
Expand Down
55 changes: 54 additions & 1 deletion src/xmpp/iq.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ iq_handlers_init(void)
int millis = prefs_get_autoping() * 1000;
xmpp_timed_handler_add(conn, _autoping_timed_send, millis, ctx);
}
received_disco_items = FALSE;

iq_rooms_cache_clear();
iq_handlers_clear();
Expand All @@ -267,8 +268,56 @@ iq_handlers_init(void)
rooms_cache = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)xmpp_stanza_release);
}

struct iq_win_finder
{
gsize max, cur;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

max can be more descriptive? E.g. max_size

char** to_be_removed;
};

static void
_win_find(char* key,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it appears that the name mismatches the purpose, maybe rather something like _mam_win_match_add?

ProfIqHandler* handler,
struct iq_win_finder* finder)
{
if (handler->func == _mam_rsm_id_handler) {
if (finder->cur >= finder->max) {
finder->max *= 2;
finder->to_be_removed = g_realloc_n(finder->to_be_removed, finder->max, sizeof(char*));
}
finder->to_be_removed[finder->cur++] = g_strdup(key);
}
}

void
iq_handlers_remove_win(ProfWin* window)
{
log_debug("Remove window %p of type %d", window, window ? window->type : -1);
if (!window)
return;
GSList *cur = late_delivery_windows, *next;
while (cur) {
LateDeliveryUserdata* del_data = cur->data;
next = g_slist_next(cur);
if (del_data->win == (void*)window)
late_delivery_windows = g_slist_delete_link(late_delivery_windows,
cur);
cur = next;
jubalh marked this conversation as resolved.
Show resolved Hide resolved
}
struct iq_win_finder st = { 0 };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undescriptive variable name, something like matcher_data would be good enough.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also another suggestion: make a blank space between } and struct, since these are 2 different logical parts (1 part to remove window, other to remove iq handlers, maybe these should even be separated on 2 functions).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, it's a bit strange approach to make your own dynamic array and structure just for this, why not something like this for readability? (let's call it pseudocode since it's not tested)

static void
_mam_win_match_add(char* key, ProfIqHandler* handler, GArray* to_be_removed)
{
    if (handler->func == _mam_rsm_id_handler) {
        // Append the element to the array
        g_array_append_val(to_be_removed, g_strdup(key));
    }
}


    GArray* to_be_removed = g_array_new(FALSE, FALSE, sizeof(char*));

    // Iterate through id_handlers and call _mam_win_match_add for each entry to identify windows to be removed.
    g_hash_table_foreach(id_handlers, (GHFunc)_mam_win_match_add, to_be_removed);

    // Access elements using g_array_index and free each element
    for (guint n = 0; n < to_be_removed->len; ++n) {
        char* to_remove = g_array_index(to_be_removed, char*, n);
        g_hash_table_remove(id_handlers, to_remove);
    }
    g_array_free(to_be_removed, TRUE);

st.max = g_hash_table_size(id_handlers);
if (st.max == 0)
return;
st.to_be_removed = g_new(char*, st.max);
g_hash_table_foreach(id_handlers, (GHFunc)_win_find, &st);
for (gsize n = 0; n < st.cur; ++n) {
g_hash_table_remove(id_handlers, st.to_be_removed[n]);
g_free(st.to_be_removed[n]);
}
g_free(st.to_be_removed);
}

void
iq_handlers_clear()
iq_handlers_clear(void)
{
if (id_handlers) {
g_hash_table_remove_all(id_handlers);
Expand Down Expand Up @@ -2696,6 +2745,10 @@ _mam_rsm_id_handler(xmpp_stanza_t* const stanza, void* const userdata)
gboolean is_complete = g_strcmp0(xmpp_stanza_get_attribute(fin, "complete"), "true") == 0;
MamRsmUserdata* data = (MamRsmUserdata*)userdata;
ProfWin* window = (ProfWin*)data->win;
if (wins_get_num(window) == -1) {
log_error("Window %p should not get any events anymore", window);
return 0;
}

buffer_remove_entry(window->layout->buffer, 0);

Expand Down
4 changes: 2 additions & 2 deletions src/xmpp/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -1261,7 +1261,7 @@ _handle_muc_private_message(xmpp_stanza_t* const stanza)
ProfMessage* message = message_init();
message->type = PROF_MSG_TYPE_MUCPM;

const gchar* from = xmpp_stanza_get_from(stanza);
const char* from = xmpp_stanza_get_from(stanza);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are few more cases, I suggest to catch all of them, using regex: gchar.+xmpp_

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to open another PR with those changes.

if (!from) {
goto out;
}
Expand Down Expand Up @@ -1360,7 +1360,7 @@ _handle_chat(xmpp_stanza_t* const stanza, gboolean is_mam, gboolean is_carbon, c
return;
}

const gchar* from = xmpp_stanza_get_from(stanza);
const char* from = xmpp_stanza_get_from(stanza);
if (!from) {
return;
}
Expand Down
3 changes: 2 additions & 1 deletion src/xmpp/xmpp.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,8 @@ void iq_enable_carbons(void);
void iq_disable_carbons(void);
void iq_send_software_version(const char* const fulljid);
void iq_rooms_cache_clear(void);
void iq_handlers_clear();
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);
Expand Down
Loading