refactor: address PR #105 review feedback
Some checks failed
CI Code / Check spelling (pull_request) Successful in 21s
CI Code / Check coding style (pull_request) Successful in 35s
CI Code / Linux (arch) (pull_request) Failing after 2m52s
CI Code / Code Coverage (pull_request) Successful in 2m56s
CI Code / Linux (debian) (pull_request) Successful in 6m48s
CI Code / Linux (ubuntu) (pull_request) Successful in 7m11s

Apply fixes, refactors and test additions requested by reviewer on PR #105.

Fixes:
- database: warn and notify user on duplicate archive_id instead of
  silently debug-logging it (R05).
- database: add missing '[' in "[DB Migration]" log prefix (R06).
- xmpp/resource: NULL-out name/status after g_free to avoid double-free
  via roster_list.c cleanup path (R09, R23).
- common: widen strtoi_range internal storage from int to long so that
  values in (INT_MAX, LONG_MAX] are rejected as out-of-range instead of
  being silently truncated on 64-bit platforms (R25).

Refactors:
- xmpp/message: extract _receive_omemo helper, removing three copies of
  the OMEMO receive block in groupchat / MUC-PM / chat handlers (R04).
- omemo: flatten deeply nested device-list processing via guard-clause
  continues (R11).
- tools/autocomplete: merge two nested ifs into a single && condition
  (R13).
- ui/titlebar: extract _show_trust_indicator and inline _wprintw_withattr
  wrapper, collapsing three near-identical trust-indicator blocks (R22).
- config/tlscerts: drop _checked_g_strdup wrapper; g_strdup is
  NULL-safe per glib documentation (R19).
- ui/inputwin: use auto_gchar for spellcheck word instead of manual
  g_free (R20).
- tools/editor: drop outdated "Deprecated synchronous" comment that
  no longer matches the callback-based implementation (R28).

Tests:
- tests/command/cmd_ac: rename segfaults_when_empty ->
  no_segfault_when_empty; expand cycling coverage to three files plus
  backward SHIFT-TAB traversal (R16, R17).
- tests/common: add strtoi_range overflow/underflow and strtol-parsing
  consistency tests (R25).
- tests/xmpp/jid: add test for '@' inside resourcepart per RFC 6122
  section 2.4 (R26).

Misc:
- xmpp/omemo: change omemo_error_to_string return type from char* to
  gchar* for glib consistency (R01).
- subprojects/libstrophe: point wrap-git at our fork at
  git.jabber.space/devs/libstrophe-gh (R24).
- RELEASE_GUIDE: drop "Updating website" section referring to an
  upstream site that is not ours (R18).
This commit is contained in:
2026-04-24 15:13:38 +03:00
parent a5fe32b245
commit 4401e817d0
21 changed files with 181 additions and 165 deletions

View File

@@ -313,25 +313,25 @@ gboolean
strtoi_range(const char* str, int* saveptr, int min, int max, gchar** err_msg)
{
char* ptr;
int val;
long lval;
if (str == NULL) {
if (err_msg)
*err_msg = g_strdup_printf("'str' input pointer can not be NULL");
return FALSE;
}
errno = 0;
val = (int)strtol(str, &ptr, 0);
lval = strtol(str, &ptr, 0);
if (errno != 0 || *str == '\0' || *ptr != '\0') {
if (err_msg)
*err_msg = g_strdup_printf("Could not convert \"%s\" to a number.", str);
return FALSE;
} else if (val < min || val > max) {
} else if (lval < (long)min || lval > (long)max) {
if (err_msg)
*err_msg = g_strdup_printf("Value %s out of range. Must be in %d..%d.", str, min, max);
return FALSE;
}
*saveptr = val;
*saveptr = (int)lval;
return TRUE;
}

View File

@@ -237,24 +237,16 @@ _name_to_tlscert_name_(const char* in, tls_cert_name_t* out, const char* name)
}
}
static gchar*
_checked_g_strdup(const char* in)
{
if (in == NULL)
return NULL;
return g_strdup(in);
}
TLSCertificate*
tlscerts_new(const char* fingerprint_sha1, int version, const char* serialnumber, const char* subjectname,
const char* issuername, const char* notbefore, const char* notafter,
const char* key_alg, const char* signature_alg, const char* pem,
const char* fingerprint_sha256, const char* pubkey_fingerprint)
{
return _tlscerts_new(_checked_g_strdup(fingerprint_sha1), version, _checked_g_strdup(serialnumber), _checked_g_strdup(subjectname),
_checked_g_strdup(issuername), _checked_g_strdup(notbefore), _checked_g_strdup(notafter),
_checked_g_strdup(key_alg), _checked_g_strdup(signature_alg), _checked_g_strdup(pem),
_checked_g_strdup(fingerprint_sha256), _checked_g_strdup(pubkey_fingerprint));
return _tlscerts_new(g_strdup(fingerprint_sha1), version, g_strdup(serialnumber), g_strdup(subjectname),
g_strdup(issuername), g_strdup(notbefore), g_strdup(notafter),
g_strdup(key_alg), g_strdup(signature_alg), g_strdup(pem),
g_strdup(fingerprint_sha256), g_strdup(pubkey_fingerprint));
}
static void

View File

@@ -581,7 +581,8 @@ _add_to_db(ProfMessage* message, const char* type, const Jid* const from_jid, co
if (rc == SQLITE_ROW) {
log_debug("Successfully inserted message into database.");
} else if (rc == SQLITE_DONE) {
log_debug("Message already exists in database (archive_id: %s), skipping.", message->stanzaid);
log_warning("Message already exists in database (archive_id: %s), skipping.", message->stanzaid);
cons_show_error("Duplicate message detected (archive_id: %s), skipping.", message->stanzaid);
} else {
log_error("SQLite error in _add_to_db() (step): %s", sqlite3_errmsg(g_chatlog_database));
}
@@ -758,7 +759,7 @@ _migrate_to_v3(void)
cleanup:
if (SQLITE_OK != sqlite3_exec(g_chatlog_database, "ROLLBACK;", NULL, 0, &err_msg)) {
log_error("DB Migration] Unable to ROLLBACK: %s", err_msg);
log_error("[DB Migration] Unable to ROLLBACK: %s", err_msg);
if (err_msg) {
sqlite3_free(err_msg);
}

View File

@@ -525,25 +525,29 @@ omemo_set_device_list(const char* const from, GList* device_list)
}
}
if (!found) {
auto_gchar gchar* dev_id_str = g_strdup_printf("%u", dev_id);
if (!g_key_file_has_key(omemo_ctx.knowndevices.keyfile, jid->barejid, dev_id_str, NULL)) {
if (equals_our_barejid(jid->barejid)) {
if (dev_id != omemo_ctx.device_id) {
cons_show("New OMEMO device (ID: %u) added to your account.", dev_id);
}
} else {
ProfChatWin* chatwin = wins_get_chat(jid->barejid);
if (chatwin) {
win_println((ProfWin*)chatwin, THEME_DEFAULT, "!", "New OMEMO device (ID: %u) found for %s.", dev_id, jid->barejid);
} else {
ProfMucWin* mucwin = wins_get_muc(jid->barejid);
if (mucwin) {
win_println((ProfWin*)mucwin, THEME_DEFAULT, "!", "New OMEMO device (ID: %u) found for %s.", dev_id, jid->barejid);
}
}
}
if (found)
continue;
auto_gchar gchar* dev_id_str = g_strdup_printf("%u", dev_id);
if (g_key_file_has_key(omemo_ctx.knowndevices.keyfile, jid->barejid, dev_id_str, NULL))
continue;
if (equals_our_barejid(jid->barejid)) {
if (dev_id != omemo_ctx.device_id) {
cons_show("New OMEMO device (ID: %u) added to your account.", dev_id);
}
continue;
}
ProfChatWin* chatwin = wins_get_chat(jid->barejid);
if (chatwin) {
win_println((ProfWin*)chatwin, THEME_DEFAULT, "!", "New OMEMO device (ID: %u) found for %s.", dev_id, jid->barejid);
continue;
}
ProfMucWin* mucwin = wins_get_muc(jid->barejid);
if (mucwin) {
win_println((ProfWin*)mucwin, THEME_DEFAULT, "!", "New OMEMO device (ID: %u) found for %s.", dev_id, jid->barejid);
}
}
}

View File

@@ -399,10 +399,9 @@ _search(Autocomplete ac, GList* curr, gboolean quote, search_direction direction
// We only use transliterated match if the search string conversion didn't result
// in unknown characters ('?'), to avoid false matches between different scripts.
if (strchr(search_str_ascii_lower, '?') == NULL) {
if (g_str_has_prefix(curr_ascii_lower, search_str_ascii_lower)) {
match = TRUE;
}
if (strchr(search_str_ascii_lower, '?') == NULL
&& g_str_has_prefix(curr_ascii_lower, search_str_ascii_lower)) {
match = TRUE;
}
}

View File

@@ -246,9 +246,8 @@ editor_process(ProfWin* window)
log_debug("[Editor] Exiting background mode");
}
// Deprecated synchronous editor call. Returns a message as returned_message.
// Please avoid using it, since it blocks execution of the message handling and other important functionality until the editor is closed.
// Returns true if an error occurred
// Returns a message via callback once the editor is closed.
// Returns true if an error occurred.
gboolean
launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* data), void* user_data)
{

View File

@@ -417,9 +417,8 @@ _inp_write(char* line, int offset)
end += next_ch_len;
}
char* word = g_strndup(&line[start], end - start);
auto_gchar gchar* word = g_strndup(&line[start], end - start);
gboolean misspelled = spellcheck_is_misspelled(word);
g_free(word);
if (misspelled) {
wattron(inp_win, theme_attrs(THEME_INPUT_MISSPELLED));

View File

@@ -45,6 +45,24 @@ static void _show_privacy(ProfChatWin* chatwin);
static void _show_muc_privacy(ProfMucWin* mucwin);
static void _show_scrolled(ProfWin* current);
static void _show_attention(ProfWin* current, gboolean attention);
static void _show_trust_indicator(gboolean trusted, int bracket_attrs, int trusted_attrs, int untrusted_attrs);
static inline void
_wprintw_withattr(WINDOW* w, const char* text, int attrs)
{
wattron(w, attrs);
wprintw(w, "%s", text);
wattroff(w, attrs);
}
static void
_show_trust_indicator(gboolean trusted, int bracket_attrs, int trusted_attrs, int untrusted_attrs)
{
wprintw(win, " ");
_wprintw_withattr(win, "[", bracket_attrs);
_wprintw_withattr(win, trusted ? "trusted" : "untrusted", trusted ? trusted_attrs : untrusted_attrs);
_wprintw_withattr(win, "]", bracket_attrs);
}
void
create_title_bar(void)
@@ -433,29 +451,7 @@ _show_muc_privacy(ProfMucWin* mucwin)
wattroff(win, bracket_attrs);
#ifdef HAVE_OMEMO
if (mucwin->omemo_trusted) {
wprintw(win, " ");
wattron(win, bracket_attrs);
wprintw(win, "[");
wattroff(win, bracket_attrs);
wattron(win, trusted_attrs);
wprintw(win, "trusted");
wattroff(win, trusted_attrs);
wattron(win, bracket_attrs);
wprintw(win, "]");
wattroff(win, bracket_attrs);
} else {
wprintw(win, " ");
wattron(win, bracket_attrs);
wprintw(win, "[");
wattroff(win, bracket_attrs);
wattron(win, untrusted_attrs);
wprintw(win, "untrusted");
wattroff(win, untrusted_attrs);
wattron(win, bracket_attrs);
wprintw(win, "]");
wattroff(win, bracket_attrs);
}
_show_trust_indicator(mucwin->omemo_trusted, bracket_attrs, trusted_attrs, untrusted_attrs);
#endif
return;
@@ -524,29 +520,7 @@ _show_privacy(ProfChatWin* chatwin)
wattron(win, bracket_attrs);
wprintw(win, "]");
wattroff(win, bracket_attrs);
if (chatwin->otr_is_trusted) {
wprintw(win, " ");
wattron(win, bracket_attrs);
wprintw(win, "[");
wattroff(win, bracket_attrs);
wattron(win, trusted_attrs);
wprintw(win, "trusted");
wattroff(win, trusted_attrs);
wattron(win, bracket_attrs);
wprintw(win, "]");
wattroff(win, bracket_attrs);
} else {
wprintw(win, " ");
wattron(win, bracket_attrs);
wprintw(win, "[");
wattroff(win, bracket_attrs);
wattron(win, untrusted_attrs);
wprintw(win, "untrusted");
wattroff(win, untrusted_attrs);
wattron(win, bracket_attrs);
wprintw(win, "]");
wattroff(win, bracket_attrs);
}
_show_trust_indicator(chatwin->otr_is_trusted, bracket_attrs, trusted_attrs, untrusted_attrs);
return;
}
@@ -588,29 +562,7 @@ _show_privacy(ProfChatWin* chatwin)
wattroff(win, bracket_attrs);
#ifdef HAVE_OMEMO
if (chatwin->omemo_trusted) {
wprintw(win, " ");
wattron(win, bracket_attrs);
wprintw(win, "[");
wattroff(win, bracket_attrs);
wattron(win, trusted_attrs);
wprintw(win, "trusted");
wattroff(win, trusted_attrs);
wattron(win, bracket_attrs);
wprintw(win, "]");
wattroff(win, bracket_attrs);
} else {
wprintw(win, " ");
wattron(win, bracket_attrs);
wprintw(win, "[");
wattroff(win, bracket_attrs);
wattron(win, untrusted_attrs);
wprintw(win, "untrusted");
wattroff(win, untrusted_attrs);
wattron(win, bracket_attrs);
wprintw(win, "]");
wattroff(win, bracket_attrs);
}
_show_trust_indicator(chatwin->omemo_trusted, bracket_attrs, trusted_attrs, untrusted_attrs);
#endif
return;

View File

@@ -65,11 +65,30 @@ static void _handle_pubsub(xmpp_stanza_t* const stanza, xmpp_stanza_t* const eve
static gboolean _handle_form(xmpp_stanza_t* const stanza);
static gboolean _handle_jingle_message(xmpp_stanza_t* const stanza);
static gboolean _should_ignore_based_on_silence(xmpp_stanza_t* const stanza);
#ifdef HAVE_OMEMO
static void _receive_omemo(xmpp_stanza_t* const stanza, ProfMessage* message);
#endif
#ifdef HAVE_LIBGPGME
static xmpp_stanza_t* _ox_openpgp_signcrypt(xmpp_ctx_t* ctx, const char* const to, const char* const text);
#endif // HAVE_LIBGPGME
#ifdef HAVE_OMEMO
static void
_receive_omemo(xmpp_stanza_t* const stanza, ProfMessage* message)
{
message->plain = omemo_receive_message(stanza, &message->trusted, &message->omemo_err);
if (message->omemo_err != OMEMO_ERR_NONE) {
message->enc = PROF_MSG_ENC_OMEMO;
if (message->plain == NULL) {
message->plain = omemo_error_to_string(message->omemo_err);
}
} else if (message->plain != NULL) {
message->enc = PROF_MSG_ENC_OMEMO;
}
}
#endif // HAVE_OMEMO
static GHashTable* pubsub_event_handlers;
gchar*
@@ -1088,15 +1107,7 @@ _handle_groupchat(xmpp_stanza_t* const stanza)
// check omemo encryption
#ifdef HAVE_OMEMO
message->plain = omemo_receive_message(stanza, &message->trusted, &message->omemo_err);
if (message->omemo_err != OMEMO_ERR_NONE) {
message->enc = PROF_MSG_ENC_OMEMO;
if (message->plain == NULL) {
message->plain = omemo_error_to_string(message->omemo_err);
}
} else if (message->plain != NULL) {
message->enc = PROF_MSG_ENC_OMEMO;
}
_receive_omemo(stanza, message);
#endif
if (!message->plain && !message->body) {
@@ -1253,15 +1264,7 @@ _handle_muc_private_message(xmpp_stanza_t* const stanza)
// check omemo encryption
#ifdef HAVE_OMEMO
message->plain = omemo_receive_message(stanza, &message->trusted, &message->omemo_err);
if (message->omemo_err != OMEMO_ERR_NONE) {
message->enc = PROF_MSG_ENC_OMEMO;
if (message->plain == NULL) {
message->plain = omemo_error_to_string(message->omemo_err);
}
} else if (message->plain != NULL) {
message->enc = PROF_MSG_ENC_OMEMO;
}
_receive_omemo(stanza, message);
#endif
message->timestamp = stanza_get_delay(stanza);
@@ -1428,15 +1431,7 @@ _handle_chat(xmpp_stanza_t* const stanza, gboolean is_mam, gboolean is_carbon, c
#ifdef HAVE_OMEMO
// check omemo encryption
message->plain = omemo_receive_message(stanza, &message->trusted, &message->omemo_err);
if (message->omemo_err != OMEMO_ERR_NONE) {
message->enc = PROF_MSG_ENC_OMEMO;
if (message->plain == NULL) {
message->plain = omemo_error_to_string(message->omemo_err);
}
} else if (message->plain != NULL) {
message->enc = PROF_MSG_ENC_OMEMO;
}
_receive_omemo(stanza, message);
#endif
xmpp_stanza_t* encrypted = xmpp_stanza_get_child_by_ns(stanza, STANZA_NS_ENCRYPTED);

View File

@@ -708,7 +708,7 @@ _omemo_bundle_publish_configure_result(xmpp_stanza_t* const stanza, void* const
return 0;
}
char*
gchar*
omemo_error_to_string(omemo_error_t error)
{
switch (error) {

View File

@@ -19,4 +19,4 @@ void omemo_bundle_request(const char* const jid, uint32_t device_id, ProfIqCallb
int omemo_start_device_session_handle_bundle(xmpp_stanza_t* const stanza, void* const userdata);
char* omemo_receive_message(xmpp_stanza_t* const stanza, gboolean* trusted, omemo_error_t* error) __attribute__((nonnull(2, 3)));
char* omemo_error_to_string(omemo_error_t error);
gchar* omemo_error_to_string(omemo_error_t error);

View File

@@ -70,7 +70,9 @@ resource_destroy(Resource* resource)
{
if (resource) {
g_free(resource->name);
resource->name = NULL;
g_free(resource->status);
resource->status = NULL;
g_free(resource);
}
}