[#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
9 changed files with 259 additions and 50 deletions
Showing only changes of commit 9b292a6100 - Show all commits

View File

@@ -2988,8 +2988,19 @@ command_docgen(void)
cmds = g_list_insert_sorted(cmds, (gpointer)pcmd, (GCompareFunc)_cmp_command);
}
FILE* toc_fragment = fopen("toc_fragment.html", "w");
FILE* main_fragment = fopen("main_fragment.html", "w");
FILE* toc_fragment = fopen("toc_fragment.html", "w");
if (!toc_fragment) {
log_error("command_docgen(): unable to open toc_fragment.html for writing: %s", g_strerror(errno));
g_list_free(cmds);
return;
}
FILE* main_fragment = fopen("main_fragment.html", "w");
if (!main_fragment) {
log_error("command_docgen(): unable to open main_fragment.html for writing: %s", g_strerror(errno));
fclose(toc_fragment);
g_list_free(cmds);
return;
}
fputs("<ul><li><ul><li>\n", toc_fragment);
fputs("<hr>\n", main_fragment);
@@ -3057,8 +3068,8 @@ command_docgen(void)
fputs("</ul></ul>\n", toc_fragment);
fclose(toc_fragment);
fclose(main_fragment);
fclose(toc_fragment);
fclose(main_fragment);
printf("\nProcessed %d commands.\n\n", g_list_length(cmds));
g_list_free(cmds);
}
@@ -3093,7 +3104,12 @@ command_mangen(void)
log_error("command_mangen(): could not allocate memory");
return;
}
FILE* manpage = fopen(filename, "w");
FILE* manpage = fopen(filename, "w");
if (!manpage) {
log_error("command_mangen(): unable to open %s for writing: %s", filename, g_strerror(errno));
curr = g_list_next(curr);
continue;
}
fprintf(manpage, "%s\n", header);
fputs(".SH NAME\n", manpage);
@@ -3128,7 +3144,7 @@ command_mangen(void)
}
}
fclose(manpage);
fclose(manpage);
curr = g_list_next(curr);
}

View File

@@ -65,6 +65,36 @@ static gboolean _check_available_space_for_db_migration(char* path_to_db);
static const int latest_version = 2;
// Helper: close DB handle (if any), warn on busy, and shutdown SQLite
static void
_db_teardown(const char* ctx)
{
if (g_chatlog_database) {
int rc = sqlite3_close_v2(g_chatlog_database);
if (rc != SQLITE_OK) {
log_warning("sqlite3_close_v2 in %s returned %d; database may still have active statements.",
ctx ? ctx : "db_teardown", rc);
}
g_chatlog_database = NULL;
}
sqlite3_shutdown();
jabber.developer marked this conversation as resolved Outdated

Earlier it was conditional. Could it cause an issue to have unconditional sqlite3_shutdown?

Also the condition takes almost full function. Maybe it would make sense to use early exit statement if (!g_chatlog_database) return;?

Earlier it was conditional. Could it cause an issue to have unconditional sqlite3_shutdown? Also the condition takes almost full function. Maybe it would make sense to use early exit statement `if (!g_chatlog_database) return;`?

Unconditional sqlite3_shutdown() is safe — it's a no-op if SQLite is already shut down.

https://www.sqlite.org/c3ref/initialize.html

Unconditional sqlite3_shutdown() is safe — it's a no-op if SQLite is already shut down. https://www.sqlite.org/c3ref/initialize.html
}
// Helper: prepare a statement and log a contextual error on failure
static gboolean
_db_prepare_ctx(const char* query, sqlite3_stmt** stmt, const char* ctx)
{
int rc = sqlite3_prepare_v2(g_chatlog_database, query, -1, stmt, NULL);
if (rc != SQLITE_OK) {
log_error("SQLite error in %s: (error code: %d) %s",
ctx ? ctx : "sqlite3_prepare_v2",
rc,
sqlite3_errmsg(g_chatlog_database));
return FALSE;
}
return TRUE;
}
static char*
_db_strdup(const char* str)
{
@@ -106,11 +136,7 @@ log_database_init(ProfAccount* account)
if (ret != SQLITE_OK) {
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();
_db_teardown("log_database_init(open)");
return FALSE;
}
@@ -222,11 +248,7 @@ 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();
_db_teardown("log_database_init(out)");
return FALSE;
}
@@ -234,14 +256,7 @@ void
log_database_close(void)
{
log_debug("log_database_close() called");
if (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;
}
_db_teardown("log_database_close");
}
void
@@ -320,9 +335,7 @@ log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_las
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_limits_info().");
if (!_db_prepare_ctx(query, &stmt, "log_database_get_limits_info()")) {
if (is_last) {
msg->timestamp = g_date_time_new_now_utc();
jabber.developer marked this conversation as resolved Outdated

Analogous to the previous comment.

Analogous to the previous comment.
}
@@ -387,9 +400,7 @@ log_database_get_previous_chat(const gchar* const contact_barejid, const gchar*
return DB_RESPONSE_ERROR;
}
int rc = sqlite3_prepare_v2(g_chatlog_database, query, -1, &stmt, NULL);
if (rc != SQLITE_OK) {
log_error("SQLite error in log_database_get_previous_chat(): (error code: %d) %s", rc, sqlite3_errmsg(g_chatlog_database));
if (!_db_prepare_ctx(query, &stmt, "log_database_get_previous_chat()")) {
return DB_RESPONSE_ERROR;
}
@@ -533,9 +544,7 @@ _add_to_db(ProfMessage* message, char* type, const Jid* const from_jid, const Ji
}
sqlite3_stmt* lmc_stmt = NULL;
if (SQLITE_OK != sqlite3_prepare_v2(g_chatlog_database, replace_check_query, -1, &lmc_stmt, NULL)) {
log_error("SQLite error in _add_to_db() on selecting original message: %s", sqlite3_errmsg(g_chatlog_database));
if (!_db_prepare_ctx(replace_check_query, &lmc_stmt, "_add_to_db(replace_check)")) {
return;
}
@@ -572,8 +581,7 @@ _add_to_db(ProfMessage* message, char* type, const Jid* const from_jid, const Ji
}
sqlite3_stmt* stmt;
if (SQLITE_OK == sqlite3_prepare_v2(g_chatlog_database, duplicate_check_query, -1, &stmt, NULL)) {
if (_db_prepare_ctx(duplicate_check_query, &stmt, "_add_to_db(duplicate_check)")) {
if (sqlite3_step(stmt) == SQLITE_ROW) {
log_error("Duplicate stanza-id found for the message. stanza_id: %s; archive_id: %s; sender: %s; content: %s", message->id, message->stanzaid, from_jid->barejid, message->plain);
cons_show_error("Got a message with duplicate (server-generated) stanza-id from %s.", from_jid->fulljid);
@@ -629,8 +637,7 @@ _get_db_version(void)
int current_version = -1;
const char* query = "SELECT `version` FROM `DbVersion` LIMIT 1";
sqlite3_stmt* statement;
if (sqlite3_prepare_v2(g_chatlog_database, query, -1, &statement, NULL) == SQLITE_OK) {
if (_db_prepare_ctx(query, &statement, "_get_db_version()")) {
if (sqlite3_step(statement) == SQLITE_ROW) {
current_version = sqlite3_column_int(statement, 0);
}

View File

@@ -144,10 +144,13 @@ void
create_input_window(void)
{
/* MB_CUR_MAX is evaluated at runtime depending on the current
* locale, therefore we check that our own version is big enough
* and bail out if it isn't.
* locale; ensure our own compiled-in maximum is sufficient.
* Fail gracefully instead of aborting in production.
*/
assert(MB_CUR_MAX <= PROF_MB_CUR_MAX);
if (MB_CUR_MAX > PROF_MB_CUR_MAX) {
cons_show_error("Your locale's MB_CUR_MAX (%zu) exceeds PROF_MB_CUR_MAX (%d); input window disabled.", (size_t)MB_CUR_MAX, PROF_MB_CUR_MAX);
jabber.developer marked this conversation as resolved Outdated

Technically correct, but what user is supposed to do with this data? Can we provide some actionable advise or explanation?

Technically correct, but what user is supposed to do with this data? Can we provide some actionable advise or explanation?

Comment added

Comment added
return;
}
#ifdef NCURSES_REENTRANT
set_escdelay(25);
#else

View File

@@ -75,6 +75,21 @@ static void _win_print_internal(ProfWin* window, const char* show_char, int pad_
int flags, theme_item_t theme_item, const char* const from, const char* const message, DeliveryReceipt* receipt);
static void _win_print_wrapped(WINDOW* win, const char* const message, size_t indent, int pad_indent);
// Helper: clamp a subwindow width to a sane range [1, cols-1] if possible
static int
_check_subwin_width(int cols, int width)
{
if (cols > 1) {
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
if (width < 1)
width = 1;
if (width >= cols)
width = cols - 1;
} else {
width = 1;
}
return width;
}
int
win_roster_cols(void)
{
@@ -82,12 +97,7 @@ win_roster_cols(void)
int cols = getmaxx(stdscr);
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;
}
width = _check_subwin_width(cols, width);
return width;
}
@@ -98,12 +108,7 @@ win_occpuants_cols(void)
int cols = getmaxx(stdscr);
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;
}
width = _check_subwin_width(cols, width);
return width;
}