[#49] harden NULL handling and resource lifecycle across UI and SQLite #47

Manually merged
jabber.developer merged 7 commits from playground/fix/src_refactoring into master 2026-02-06 19:01:48 +00:00
8 changed files with 203 additions and 25 deletions
Showing only changes of commit a9c21ce487 - Show all commits

View File

@@ -98,17 +98,23 @@ log_database_init(ProfAccount* account)
auto_char char* filename = _get_db_filename(account);
if (!filename) {
sqlite3_shutdown();
return FALSE;
}
ret = sqlite3_open(filename, &g_chatlog_database);
if (ret != SQLITE_OK) {
const char* err_msg = sqlite3_errmsg(g_chatlog_database);
const char* err_msg = g_chatlog_database ? sqlite3_errmsg(g_chatlog_database) : "(no handle)";
log_error("Error opening SQLite database: %s", err_msg);
if (g_chatlog_database) {
sqlite3_close(g_chatlog_database);
g_chatlog_database = NULL;
}
sqlite3_shutdown();
return FALSE;
}
char* err_msg;
char* err_msg = NULL;
int db_version = _get_db_version();
if (db_version == latest_version) {
@@ -216,6 +222,11 @@ out:
} else {
log_error("Unknown SQLite error in log_database_init().");
}
if (g_chatlog_database) {
sqlite3_close(g_chatlog_database);
g_chatlog_database = NULL;
}
sqlite3_shutdown();
return FALSE;
}
@@ -224,7 +235,10 @@ log_database_close(void)
{
log_debug("log_database_close() called");
if (g_chatlog_database) {
sqlite3_close(g_chatlog_database);
int rc = sqlite3_close_v2(g_chatlog_database);
if (rc != SQLITE_OK) {
log_warning("sqlite3_close_v2 returned %d; database may still have active statements.", rc);
}
sqlite3_shutdown();
g_chatlog_database = NULL;
}
@@ -281,8 +295,15 @@ log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_las
{
sqlite3_stmt* stmt = NULL;
const Jid* myjid = connection_get_jid();
if (!myjid->str)
return NULL;
// Always return a valid ProfMessage to avoid NULL dereferences in callers
ProfMessage* msg = message_init();
if (!myjid || !myjid->str) {
// If caller requested the last message and we have no context, fall back to now
if (is_last) {
msg->timestamp = g_date_time_new_now_utc();
}
return msg;
}
const char* order = is_last ? "DESC" : "ASC";
auto_sqlite char* query = sqlite3_mprintf("SELECT `archive_id`, `timestamp` FROM `ChatLogs` WHERE "
@@ -293,17 +314,21 @@ log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_las
if (!query) {
log_error("Could not allocate memory for SQL query in log_database_get_limits_info()");
return NULL;
if (is_last) {
msg->timestamp = g_date_time_new_now_utc();
jabber.developer marked this conversation as resolved Outdated

Wouldn't this cause a faulty logic on iq_mam_request_older?

src/xmpp/iq.c Lines 2654 to 2660 in 74cfd32c1e
// If first message found
if (first_msg->timestamp) {
firstid = first_msg->stanzaid;
enddate = g_date_time_format(first_msg->timestamp, mam_timestamp_format_string);
} else {
return;
}

Wouldn't this cause a faulty logic on `iq_mam_request_older`? https://git.jabber.space/devs/cproof/src/commit/74cfd32c1ec981f3ad186fbee998c10a473df45b/src/xmpp/iq.c#L2654-L2660
}
return msg;
}
int rc = sqlite3_prepare_v2(g_chatlog_database, query, -1, &stmt, NULL);
if (rc != SQLITE_OK) {
log_error("Unknown SQLite error in log_database_get_last_info().");
return NULL;
log_error("Unknown SQLite error in log_database_get_limits_info().");
if (is_last) {
msg->timestamp = g_date_time_new_now_utc();
}
return msg;
}
ProfMessage* msg = message_init();
if (sqlite3_step(stmt) == SQLITE_ROW) {
char* archive_id = (char*)sqlite3_column_text(stmt, 0);
char* date = (char*)sqlite3_column_text(stmt, 1);
@@ -313,6 +338,11 @@ log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_las
}
sqlite3_finalize(stmt);
jabber.developer marked this conversation as resolved Outdated

Analogous to the previous comment.

Analogous to the previous comment.
// If nothing was found and caller expects the last message, provide a sane default
if (!msg->timestamp && is_last) {
msg->timestamp = g_date_time_new_now_utc();
}
return msg;
}

View File

@@ -140,6 +140,10 @@ void
buffer_remove_entry(ProfBuff buffer, int entry)
{
GSList* node = g_slist_nth(buffer->entries, entry);
if (node == NULL) {
// Index out of range; nothing to remove
return;
}
ProfBuffEntry* e = node->data;
buffer->lines -= e->_lines;
_free_entry(e);
@@ -168,6 +172,9 @@ ProfBuffEntry*
buffer_get_entry(ProfBuff buffer, int entry)
{
GSList* node = g_slist_nth(buffer->entries, entry);
if (node == NULL) {
return NULL;
}
return node->data;
}
@@ -189,6 +196,7 @@ buffer_get_entry_by_id(ProfBuff buffer, const char* const id)
static ProfBuffEntry*
_create_entry(const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const from_jid, const char* const message, DeliveryReceipt* receipt, const char* const id, int y_start_pos, int y_end_pos)
{
assert(time != NULL);
ProfBuffEntry* e = malloc(sizeof(struct prof_buff_entry_t));
e->show_char = STRDUP_OR_NULL(show_char);
e->pad_indent = pad_indent;

View File

@@ -592,8 +592,11 @@ chatwin_db_history(ProfChatWin* chatwin, const gchar* start_time, const gchar* e
{
auto_gchar gchar* _end_time = NULL;
if (!end_time && buffer_size(((ProfWin*)chatwin)->layout->buffer) > 0) {
_end_time = g_date_time_format_iso8601(buffer_get_entry(((ProfWin*)chatwin)->layout->buffer, 0)->time);
end_time = _end_time;
ProfBuffEntry* first = buffer_get_entry(((ProfWin*)chatwin)->layout->buffer, 0);
if (first && first->time) {
_end_time = g_date_time_format_iso8601(first->time);
end_time = _end_time;
}
}
GSList* history = NULL;

View File

@@ -163,6 +163,10 @@ create_input_window(void)
rl_callback_handler_install(NULL, _inp_rl_linehandler);
inp_win = newpad(1, INP_WIN_MAX);
if (!inp_win) {
// Failed to allocate input pad; leave inp_win NULL and avoid further use
return;
}
wbkgd(inp_win, theme_attrs(THEME_INPUT_TEXT));
jabber.developer marked this conversation as resolved Outdated

Maybe we should add error logging for gracefulness?

Maybe we should add error logging for gracefulness?

Not resolved; no response.

Not resolved; no response.

Added

Added
keypad(inp_win, TRUE);
wmove(inp_win, 0, 0);
@@ -238,6 +242,9 @@ inp_readline(void)
void
inp_win_resize(void)
{
if (!inp_win) {
return;
}
int col = getcurx(inp_win);
int wcols = getmaxx(stdscr);
@@ -285,8 +292,10 @@ void
inp_close(void)
{
rl_callback_handler_remove();
delwin(inp_win);
inp_win = NULL;
if (inp_win) {
delwin(inp_win);
inp_win = NULL;
}
fclose(discard);
discard = NULL;
}
@@ -294,6 +303,9 @@ inp_close(void)
char*
inp_get_line(void)
{
if (!inp_win) {
return NULL;
}
werase(inp_win);
wmove(inp_win, 0, 0);
_inp_win_update_virtual();
@@ -318,6 +330,9 @@ inp_set_line(const char* const new_line)
char*
inp_get_password(void)
{
if (!inp_win) {
return NULL;
}
werase(inp_win);
wmove(inp_win, 0, 0);
_inp_win_update_virtual();

View File

@@ -111,16 +111,23 @@ status_bar_init(void)
int row = screen_statusbar_row();
int cols = getmaxx(stdscr);
if (cols <= 0) {
jabber.developer marked this conversation as resolved Outdated

Is it realistic condition? It doesn't appear a normal state, so maybe we should log_warn it?

Is it realistic condition? It doesn't appear a normal state, so maybe we should log_warn it?

Not resolved, as no response given/action taken.

Not resolved, as no response given/action taken.

Added

Added
cols = 1;
}
statusbar_win = newwin(1, cols, row, 0);
status_bar_draw();
if (statusbar_win) {
status_bar_draw();
}
}
void
status_bar_close(void)
{
delwin(statusbar_win);
statusbar_win = NULL;
if (statusbar_win) {
delwin(statusbar_win);
statusbar_win = NULL;
}
if (statusbar) {
if (statusbar->time) {
g_free(statusbar->time);
@@ -145,7 +152,13 @@ status_bar_close(void)
void
status_bar_resize(void)
{
if (!statusbar_win) {
return;
}
int cols = getmaxx(stdscr);
if (cols <= 0) {
cols = 1;
}
werase(statusbar_win);
int row = screen_statusbar_row();
wresize(statusbar_win, 1, cols);
@@ -285,6 +298,9 @@ status_bar_clear_fulljid(void)
void
status_bar_draw(void)
{
if (!statusbar_win) {
return;
}
werase(statusbar_win);
wbkgd(statusbar_win, theme_attrs(THEME_STATUS_TEXT));
@@ -674,8 +690,13 @@ _display_name(StatusBarTab* tab)
fullname = g_strconcat(mucwin_title, " conf", NULL);
} else if (tab->window_type == WIN_PRIVATE) {
auto_jid Jid* jid = jid_create(tab->identifier);
auto_gchar gchar* mucwin_title = mucwin_generate_title(jid->barejid, PREF_STATUSBAR_ROOM_TITLE);
fullname = g_strconcat(mucwin_title, "/", jid->resourcepart, NULL);
if (jid) {
jabber.developer marked this conversation as resolved Outdated

Nice catch!

Nice catch!
auto_gchar gchar* mucwin_title = mucwin_generate_title(jid->barejid, PREF_STATUSBAR_ROOM_TITLE);
fullname = g_strconcat(mucwin_title, "/", jid->resourcepart, NULL);
} else {
// Fallback: use identifier directly if JID parsing failed
fullname = strdup(tab->identifier);
}
} else {
fullname = strdup("window");
}

View File

@@ -73,9 +73,16 @@ void
create_title_bar(void)
{
int cols = getmaxx(stdscr);
if (cols <= 0) {
cols = 1;
}
int row = screen_titlebar_row();
win = newwin(1, cols, row, 0);
if (!win) {
// Failed to create title bar window; skip initialization to avoid NULL deref
return;
}
wbkgd(win, theme_attrs(THEME_TITLE_TEXT));
title_bar_console();
title_bar_set_presence(CONTACT_OFFLINE);
@@ -88,13 +95,18 @@ create_title_bar(void)
void
free_title_bar(void)
{
delwin(win);
win = NULL;
if (win) {
delwin(win);
win = NULL;
}
}
void
title_bar_update_virtual(void)
{
if (!win) {
return;
}
ProfWin* window = wins_get_current();
if (window->type != WIN_CONSOLE) {
if (typing_elapsed) {
@@ -114,7 +126,13 @@ title_bar_update_virtual(void)
void
title_bar_resize(void)
{
if (!win) {
return;
}
int cols = getmaxx(stdscr);
if (cols <= 0) {
cols = 1;
}
werase(win);
@@ -131,6 +149,9 @@ title_bar_resize(void)
void
title_bar_console(void)
{
if (!win) {
return;
}
werase(win);
if (typing_elapsed) {
g_timer_destroy(typing_elapsed);
@@ -192,6 +213,9 @@ title_bar_set_typing(gboolean is_typing)
static void
_title_bar_draw(void)
{
if (!win) {
return;
}
int pos;
int maxrightpos;
ProfWin* current = wins_get_current();

View File

@@ -80,7 +80,15 @@ win_roster_cols(void)
{
int roster_win_percent = prefs_get_roster_size();
int cols = getmaxx(stdscr);
jabber.developer marked this conversation as resolved Outdated

Why not like this?

return cols <= 1 ? 1 : CLAMP(width, 1, cols - 1);
Why not like this? ```c return cols <= 1 ? 1 : CLAMP(width, 1, cols - 1); ```

Corrected

Corrected
return CEILING((((double)cols) / 100) * roster_win_percent);
int width = CEILING((((double)cols) / 100) * roster_win_percent);
// Clamp to a sane range to avoid zero/full-width pads
if (cols > 1) {
if (width < 1) width = 1;
if (width >= cols) width = cols - 1;
} else {
width = 1;
}
return width;
}
int
@@ -88,7 +96,15 @@ win_occpuants_cols(void)
{
int occupants_win_percent = prefs_get_occupants_size();
int cols = getmaxx(stdscr);
return CEILING((((double)cols) / 100) * occupants_win_percent);
int width = CEILING((((double)cols) / 100) * occupants_win_percent);
// Clamp to a sane range to avoid zero/full-width pads
if (cols > 1) {
if (width < 1) width = 1;
if (width >= cols) width = cols - 1;
} else {
width = 1;
}
return width;
}
static ProfLayout*
@@ -144,6 +160,7 @@ win_create_console(void)
ProfWin*
win_create_chat(const char* const barejid)
{
assert(barejid != NULL);
ProfChatWin* new_win = malloc(sizeof(ProfChatWin));
new_win->window.type = WIN_CHAT;
new_win->window.scroll_state = WIN_SCROLL_INNER;
@@ -175,6 +192,7 @@ win_create_chat(const char* const barejid)
ProfWin*
win_create_muc(const char* const roomjid)
{
assert(roomjid != NULL);
ProfMucWin* new_win = malloc(sizeof(ProfMucWin));
int cols = getmaxx(stdscr);
@@ -233,6 +251,8 @@ win_create_muc(const char* const roomjid)
ProfWin*
win_create_config(const char* const roomjid, DataForm* form, ProfConfWinCallback submit, ProfConfWinCallback cancel, const void* userdata)
{
assert(roomjid != NULL);
assert(form != NULL);
ProfConfWin* new_win = malloc(sizeof(ProfConfWin));
new_win->window.type = WIN_CONFIG;
new_win->window.scroll_state = WIN_SCROLL_INNER;
@@ -251,6 +271,7 @@ win_create_config(const char* const roomjid, DataForm* form, ProfConfWinCallback
ProfWin*
win_create_private(const char* const fulljid)
{
assert(fulljid != NULL);
ProfPrivateWin* new_win = malloc(sizeof(ProfPrivateWin));
new_win->window.type = WIN_PRIVATE;
new_win->window.scroll_state = WIN_SCROLL_INNER;
@@ -281,6 +302,8 @@ win_create_xmlconsole(void)
ProfWin*
win_create_plugin(const char* const plugin_name, const char* const tag)
{
assert(plugin_name != NULL);
assert(tag != NULL);
ProfPluginWin* new_win = malloc(sizeof(ProfPluginWin));
new_win->window.type = WIN_PLUGIN;
new_win->window.scroll_state = WIN_SCROLL_INNER;
@@ -297,6 +320,7 @@ win_create_plugin(const char* const plugin_name, const char* const tag)
ProfWin*
win_create_vcard(vCard* vcard)
{
assert(vcard != NULL);
ProfVcardWin* new_win = malloc(sizeof(ProfVcardWin));
new_win->window.type = WIN_VCARD;
new_win->window.scroll_state = WIN_SCROLL_INNER;
@@ -348,7 +372,7 @@ win_get_title(ProfWin* window)
const ProfConfWin* confwin = (ProfConfWin*)window;
assert(confwin->memcheck == PROFCONFWIN_MEMCHECK);
auto_gchar gchar* mucwin_title = mucwin_generate_title(confwin->roomjid, PREF_TITLEBAR_MUC_TITLE);
if (confwin->form->modified) {
if (confwin->form && confwin->form->modified) {
return g_strconcat(mucwin_title, " config *", NULL);
}
return g_strconcat(mucwin_title, " config", NULL);
@@ -556,7 +580,25 @@ win_show_subwin(ProfWin* window)
}
ProfLayoutSplit* layout = (ProfLayoutSplit*)window->layout;
// If a subwindow already exists (e.g. repeated call), destroy it to avoid leaks
if (layout->subwin) {
delwin(layout->subwin);
layout->subwin = NULL;
}
// Ensure minimum width to avoid creating a zero-width pad
if (subwin_cols <= 0) {
subwin_cols = 1;
}
layout->subwin = newpad(PAD_SIZE, subwin_cols);
if (layout->subwin == NULL) {
// Failed to allocate subwindow; keep base window resized to full width
log_error("Failed to create subwindow pad (cols=%d)", subwin_cols);
wresize(layout->base.win, PAD_SIZE, cols);
win_redraw(window);
return;
}
wbkgd(layout->subwin, theme_attrs(THEME_TEXT));
wresize(layout->base.win, PAD_SIZE, cols - subwin_cols);
win_redraw(window);
@@ -910,6 +952,11 @@ win_refresh_with_subwin(ProfWin* window)
int row_end = screen_mainwin_row_end();
ProfLayoutSplit* layout = (ProfLayoutSplit*)window->layout;
// Safety: if subwindow is not active, nothing to refresh
if (layout == NULL || layout->subwin == NULL) {
return;
}
if (window->type == WIN_MUC) {
subwin_cols = win_occpuants_cols();
} else if (window->type == WIN_CONSOLE) {
@@ -2038,7 +2085,14 @@ win_print_loading_history(ProfWin* window)
gboolean is_buffer_empty = buffer_size(window->layout->buffer) == 0;
if (!is_buffer_empty) {
timestamp = buffer_get_entry(window->layout->buffer, 0)->time;
ProfBuffEntry* first = buffer_get_entry(window->layout->buffer, 0);
if (first && first->time) {
timestamp = first->time;
} else {
// Fallback to current time if entry/time is unavailable
timestamp = g_date_time_new_now_local();
is_buffer_empty = TRUE; // ensure we unref fallback timestamp below
}
} else {
timestamp = g_date_time_new_now_local();
}

View File

@@ -407,8 +407,10 @@ wins_get_by_string(const char* str)
if (barejid) {
ProfChatWin* chatwin = wins_get_chat(barejid);
if (chatwin) {
free(barejid);
return (ProfWin*)chatwin;
}
free(barejid);
}
}
@@ -603,6 +605,9 @@ wins_new_xmlconsole(void)
{
int result = _wins_get_next_available_num(keys);
ProfWin* newwin = win_create_xmlconsole();
if (!newwin) {
return NULL;
}
_wins_htable_insert(windows, GINT_TO_POINTER(result), newwin);
autocomplete_add(wins_ac, "xmlconsole");
autocomplete_add(wins_close_ac, "xmlconsole");
@@ -614,6 +619,9 @@ wins_new_chat(const char* const barejid)
{
int result = _wins_get_next_available_num(keys);
ProfWin* newwin = win_create_chat(barejid);
if (!newwin) {
return NULL;
}
_wins_htable_insert(windows, GINT_TO_POINTER(result), newwin);
autocomplete_add(wins_ac, barejid);
@@ -637,6 +645,9 @@ wins_new_muc(const char* const roomjid)
{
int result = _wins_get_next_available_num(keys);
ProfWin* newwin = win_create_muc(roomjid);
if (!newwin) {
return NULL;
}
_wins_htable_insert(windows, GINT_TO_POINTER(result), newwin);
autocomplete_add(wins_ac, roomjid);
autocomplete_add(wins_close_ac, roomjid);
@@ -651,6 +662,9 @@ wins_new_config(const char* const roomjid, DataForm* form, ProfConfWinCallback s
{
int result = _wins_get_next_available_num(keys);
ProfWin* newwin = win_create_config(roomjid, form, submit, cancel, userdata);
if (!newwin) {
return NULL;
}
_wins_htable_insert(windows, GINT_TO_POINTER(result), newwin);
return newwin;
@@ -661,6 +675,9 @@ wins_new_private(const char* const fulljid)
{
int result = _wins_get_next_available_num(keys);
ProfWin* newwin = win_create_private(fulljid);
if (!newwin) {
return NULL;
}
_wins_htable_insert(windows, GINT_TO_POINTER(result), newwin);
autocomplete_add(wins_ac, fulljid);
autocomplete_add(wins_close_ac, fulljid);
@@ -675,6 +692,9 @@ wins_new_plugin(const char* const plugin_name, const char* const tag)
{
int result = _wins_get_next_available_num(keys);
ProfWin* newwin = win_create_plugin(plugin_name, tag);
if (!newwin) {
return NULL;
}
_wins_htable_insert(windows, GINT_TO_POINTER(result), newwin);
autocomplete_add(wins_ac, tag);
autocomplete_add(wins_close_ac, tag);
@@ -686,6 +706,9 @@ wins_new_vcard(vCard* vcard)
{
int result = _wins_get_next_available_num(keys);
ProfWin* newwin = win_create_vcard(vcard);
if (!newwin) {
return NULL;
}
_wins_htable_insert(windows, GINT_TO_POINTER(result), newwin);
return newwin;