merge/upstream-full #105
2 Participants
Notifications
Due Date
No due date set.
Blocks
#109 Reenable build flags
devs/cproof
Reference: devs/cproof#105
Reference in New Issue
Block a user
No description provided.
Delete Branch "merge/upstream-full"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Merge upstream profanity-im/profanity (373 commits)
Sync with upstream
profanity-im/profanity— 373 commits from 2025-04-22 to 2026-04-09.Upstream HEAD:
1ac05754be(Merge PR #2147 — omemo-heartbeats-fix)Fork point:
5385cc28beStats: 305 files changed, +10,656 / −9,354
Taken from upstream
g_new0instead ofmalloc+memset,auto_gchar/auto_gcharvmacroslaunch_editorwin_warn_needed/win_warn_sentwarning deduplication,PAD_MIN_HEIGHTdynamic padsconnection_get_available_resourcesfor querying resourcesomemo_is_device_active,omemo_is_jid_trusted_gpgme_key_get_email— parse email from UIDUNIQUEconstraint for deduplication,g_new0in database.ccommand/,config/,xmpp/,ui/,omemo/,otr/,pgp/),test_cmd_ac.c/.h, updated stubsPreserved from cproof
replace_id ?: idlogic in message/stanza/omemocmd_force_encryption,test_forced_encryption.c/.hcons_show("%s", ...))y_start_pos-based paging inwindow.cdb_history_result_treturn type,_truncate_datetime_suffix()Merge-time fixes
-Werror)cons_show(errmsg)→cons_show("%s", errmsg)!msg->timestamp→msg && !msg->timestamp(int)(maxlen + 1)%dforsize_t%zuBuild system
test_cmd_ac,test_forced_encryptionTests
stbbr_for_xmlnsfrom updated stabber (profanity-im/stabber, 4 commits March 2026), not yet available in ourdevs/stabberforkFollow-up
profanity-im/stabberupdates intodevs/stabber(addsstbbr_for_xmlns)Closes #64
GString4194298dbd/time <invalid-section>29129bf888cmd_presence()4eaa291f70freefunction. dd8da16c96Will need to fix this soon. Lets temp remove arch to get results for the other systems. Build fail on Arch: ``` src/pgp/gpg.c: In function ‘p_gpg_decrypt’: src/pgp/gpg.c:659:36: error: implicit declaration of function ‘gpgme_key_get_string_attr’ [-Wimplicit-function-declaration] 659 | const char* addr = gpgme_key_get_string_attr(key, GPGME_ATTR_EMAIL, NULL, 0); | ^~~~~~~~~~~~~~~~~~~~~~~~~ src/pgp/gpg.c:659:67: error: ‘GPGME_ATTR_EMAIL’ undeclared (first use in this function) 659 | const char* addr = gpgme_key_get_string_attr(key, GPGME_ATTR_EMAIL, NULL, 0); | ^~~~~~~~~~~~~~~~ src/pgp/gpg.c:659:67: note: each undeclared identifier is reported only once for each function it appears in make[1]: *** [Makefile:2085: src/pgp/gpg.o] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1314: all] Error 2 ```tests/prof_cmocka.h65f5883feememchr()on strings potentially shorter than 4 bytes. 7f48452d84On Cygwin, ./configure failed with: ./configure: line XXXX: syntax error near unexpected token ACX_PTHREAD([], [AC_MSG_ERROR([pthread is required])])' Root cause: ACX_PTHREAD is a legacy macro not provided by modern Autoconf; the maintained macro is AX_PTHREAD (from autoconf-archive). Changes: - Replace ACX_PTHREAD with AX_PTHREAD in configure.ac. - Rely on system autoconf-archive; do not vendor m4/ax_pthread.m4. - Keep AC_CONFIG_MACRO_DIR([m4]) (harmless even if empty). Result: - autoreconf -fi && ./configure && make succeeds on Cygwin x86_64. - Linux builds continue to work. Fixes: #2059Add /opt/homebrew/{include,lib} flags for apple silicon macs, which are not included as default search paths by Clang/GCC. (Homebrew uses /usr/local/{include,lib} on intel macs)GErrorhandling 48ac88de08/changes24c3b5d531conf_string_list_{add,remove}()implementations. b0221f6146gbooleanconsistently. 62fd40c510accounts_get_account(). 7c04b17aff/omemo genfa1c4dddc9In autotools we had `package_status` which we set to either `release` or `development`. When converting from autotools to meson we used that mechanism as well. But actually meson has a default way of handling this with the option `buildtype`. The following values are possible: debug, debugoptimized, release, plain So our `package_status = development` will now be `if get_option('buildtype') == 'debug' or get_option('buildtype') == 'debugoptimized'`. Probably we could also use `if get_option('buildtype') == 'release'. But like this the `plain` will be really plain. Usage: ``` meson setup build --buildtype=debug meson compile -C build ```prefs_changes_printoutside ofprefs_changes309c0a64a7unsignedtypes. f55f7611c2prof_date_time_format_iso8601(). 349de02b8csession already existsmessages 4253da99c5This will result in smaller and cleaner tarballs for distributions to use. Additionally it is a logical step towards moving fully to the Meson build system. While the last release provided two tarballs (one generated with Autotools and one with Meson), the next release will only provide a Meson-generated tarball. These attributes are respected by `git archive` and `meson dist` which means they will take effect for the Meson generated tarballs and the GitHub autogenerated tarballs as well. As a consequence of switching to `meson dist`, pre-generated files like `configure` will no longer be included in the tarballs. Users and distributions that still wish to use the Autotools build system from these tarballs will now need to run `./bootstrap.sh` or `autoreconf -fi` themselves, which requires automake, autoconf, and libtool to be installed. Since many distributions are slowly dropping the usage of vendor generated tarballs and are moving to checking out git tags this should not be a problem. Arch Linux is for example doing `source=("git+https://github.com/user/project.git#tag=v1.2.3")` and Debian has the `git-buildpackage` too. So both of them require require autoreconf already. Autotools support might be removed entirely in a future release since I find them much easier to edit and maintain.Merge profanity-im/profanity master (373 commits) into cproof fork. Source changes (manual merge): - src/command/: cmd_defs, cmd_funcs, cmd_ac — upstream g_new0, auto_gchar, launch_editor callback; keep our XEP-0308 LMC, force-encryption, CWE-134 - src/config/: preferences, tlscerts, account — upstream UNIQUE dedup, dynamic pad capacity; keep our db_history_result_t, scroll logic - src/database.*: upstream g_new0 memory mgmt; keep our return types, add null-check fix for msg->timestamp - src/omemo/, src/pgp/: upstream _gpgme_key_get_email, g_new0; keep our replace_id in omemo_on_message_send - src/tools/: editor, http_upload, bookmark_ignore — upstream launch_editor callback API - src/ui/: console, window, chatwin, mucwin, inputwin, buffer — upstream win_warn_needed/sent dedup, PAD_MIN_HEIGHT dynamic pads; keep our y_start_pos scroll, _truncate_datetime_suffix - src/xmpp/: message, stanza, presence, roster_list, iq, session — upstream connection_get_available_resources; keep our LMC stanza logic - src/common.c: fix format-security (cons_show) Build system: - Makefile.am: updated test paths to subdirectory structure, added test_cmd_ac, test_forced_encryption - Restored autotools files deleted by upstream meson migration: bootstrap.sh, autogen.sh, m4/ax_valgrind_check.m4, configure-debug Tests: - Unit tests: upstream unittests.c base + our forced_encryption tests (482 passed, 0 failed across all 4 configurations) - Functional tests: kept our version (93 passed, 0 failed) upstream version requires stbbr_for_xmlns not yet in our stabber fork - Updated test stubs: stub_xmpp.c, stub_omemo.c from upstream Compile fixes: - src/common.c: cons_show(errmsg) -> cons_show("%s", errmsg) - src/database.c: null-check before msg->timestamp access - src/ui/console.c: size_t -> (int) cast for format width - src/config/tlscerts.c: %d -> %zu for size_t090c147928to79832ef86c79832ef86ctoc6bc85ccd5c6bc85ccd5to4319172758Nice job. I generally tried to make less comments to avoid unnecessary increase in the workload, but there are many places which are incompatible with our code style/approach.
@@ -82,2 +49,2 @@* Update profanity_version.txt* Take results from profanity.doap and put them into xeps.htmlMake changes to the git repo including uploading the new artefacts at:https://github.com/profanity-im/profanity-im.github.ioWe have a different website. Please remove this instruction
@@ -61,2 +35,3 @@static void _save_accounts(void);static gchar*_sanitize_account_name(const char* const name)How will it reflect on existing accounts? Would it be possible to have accounts with such names previously?
Will be handled in [#112]
@@ -246,0 +241,4 @@_checked_g_strdup(const char* in){if (in == NULL)return NULL;Why though?
According to official doc,
g_strdup:Though maybe earlier glib versions are less safe...
@@ -634,0 +581,4 @@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);It deserves at least
warninglevel. We can't just silently not write message to log: the message might contain important information for user. The error also should be extremely rare and it should represent invalid state, so it makes sense tocons_show_errorit to alert the user.@@ -738,0 +709,4 @@"`timestamp` TEXT, ""`type` TEXT, ""`stanza_id` TEXT, ""`archive_id` TEXT UNIQUE, "Most of the messages will have this value as
NULL. Luckily, SQLite treats them as unique values.Just a consideration. Feel free to resolve it.
@@ -738,0 +758,4 @@cleanup:if (SQLITE_OK != sqlite3_exec(g_chatlog_database, "ROLLBACK;", NULL, 0, &err_msg)) {log_error("DB Migration] Unable to ROLLBACK: %s", err_msg);Why not?
log_error("[DB Migration] Unable to ROLLBACK: %s", err_msg);[is missing (likely a strange typo)@@ -266,3 +281,4 @@}free_keyfile(&omemo_ctx.identity);g_hash_table_destroy(omemo_ctx.known_devices);TODO: investigate double free/similar issues due to cleaning regardless of
omemo_ctx.loaded. Likely, it's intentional, but I want to make sure. double free is worse than memleakFixed in
60da899bd— NULL-checks added before eachg_hash_table_destroy/signal_*_destroy, since neither is documented as NULL-safe and a partial init leaves the handles zero.@@ -506,0 +539,4 @@} 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);extremely excessive nesting
@@ -298,2 +272,3 @@if (whitespace_base) {if (strstr(message, OTRL_MESSAGE_TAG_V2) || strstr(message, OTRL_MESSAGE_TAG_V1)) {char* tag_position = whitespace_base + strlen(OTRL_MESSAGE_TAG_BASE);if (strncmp(tag_position, OTRL_MESSAGE_TAG_V2, strlen(OTRL_MESSAGE_TAG_V2)) == 0 || strncmp(tag_position, OTRL_MESSAGE_TAG_V1, strlen(OTRL_MESSAGE_TAG_V1)) == 0) {TODO: test with other popular messenger to ensure that this logic works correctly.
Will be handled in #112 (OTR interop matrix)
@@ -409,0 +400,4 @@// 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)) {could be just
&&operator instead of nested condition@@ -234,13 +249,10 @@ editor_process(ProfWin* window)// Please avoid using it, since it blocks execution of the message handling and other important functionality until the editor is closed.need to remove this comment since it doesn't reflect new version of this method
@@ -43,3 +16,1 @@void http_print_transfer(ProfWin* window, char* id, const char* fmt, ...);G_GNUC_PRINTF(3, 4)void http_print_transfer_update(ProfWin* window, char* id, const char* fmt, ...);G_GNUC_PRINTF(4, 5)Nice catch. Feel free to close.
@@ -1014,4 +1048,1 @@auto_gchar gchar* message = NULL;get_message_from_editor_async(rl_line_buffer);note: we should also clean it up its definition later
Done — switched to
auto_gchar(4401e817d).I mean
get_message_from_editor_asyncfunction is rendered obsolete by new editor function from upsteam.Removed
@@ -404,0 +400,4 @@end += next_ch_len;}char* word = g_strndup(&line[start], end - start);It would be better to use
auto_gchar gchar* wordinstead ofg_freeing it 2 lines below, because we would maintain consistency across the code (auto_-style), avoid risks of UAF in the future, and also match the type (g_strndupreturnsgchar*, notchar*)@@ -331,4 +305,2 @@_get_range_bounds(&start, &end, is_static);pos = getmaxx(stdscr) - _tabs_width(start, end);if (pos < 0) {If it wasn't changed, it might've lead to the bug, because previous assignment would've led to overflow of the
pos, thus creating contradicting condition (pos < 0always false).fixed by precheck rewrite
@@ -87,3 +57,1 @@static int _status_bar_draw_extended_tabs(int pos, gboolean prefix, int start, int end, gboolean is_static);static int _status_bar_draw_tab(StatusBarTab* tab, int pos, int num, gboolean include_brackets);static int _status_bar_draw_tabs(int pos);void _get_range_bounds(guint* start, guint* end, gboolean is_static);All these
guintmight lead to unexpected bugs.https://google.github.io/styleguide/cppguide.html
We should discuss style guide for CProof.
Updated
@@ -335,2 +307,2 @@pos = 0;}// if the result of the calc is negative we take 0pos = MAX(0, (getmaxx(stdscr) - (int)_tabs_width(start, end)));Without
(int)typecasting,_tabs_widthalways returns result> 0. And it's so hard to maintain such part since we need to keep in mind that result is unsigned. It's a hack rather than a normal implementation. I suggest we revert these changes: they decrease maintainability, and reduce compiler's ability to detect issues.We should revert
ddd43f5e608970b5e61band other similar commits.
Type handling updated
@@ -581,2 +588,4 @@wattroff(win, bracket_attrs);#ifdef HAVE_OMEMOif (chatwin->omemo_trusted) {Literally WET. This parts repeats one-to-one with the part above. It deserves its own method. More than that,
It take too much space and reduce readability. We can make macro or at least a wrapper method to do such thing instead:
Nice refactoring.
@@ -577,27 +577,9 @@ win_show_subwin(ProfWin* window)}ProfLayoutSplit* layout = (ProfLayoutSplit*)window->layout;// If a subwindow already exists (e.g. repeated call), destroy it to avoid leaksTODO: test leaks here
Fixed in
60da899bd—win_show_subwinnowdelwins a previous pad beforenewpad, removing the leak on a duplicate show.@@ -66,0 +64,4 @@wresize(win, new_height, cur_width);}}}We already have a similar method. TODO: understand interaction of these 2 fixes.
Will be handled in #112 —
_win_ensure_pad_capacityandwin_resizecover different concerns@@ -1095,3 +1070,1 @@stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID);if (stanzaid) {message->stanzaid = strdup(stanzaid);const char* by = xmpp_stanza_get_attribute(stanzaidst, "by");I am not sure right now, but I recall one XEP mentioning that we shouldn't trust
by. Further investigation is required.Will be handled in #112 — XEP-0359 §7 + §8 require a disco check (
urn:xmpp:sid:0) before trusting thebyattribute; current code only matches the JID.@@ -1407,3 +1395,1 @@stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID);if (stanzaid) {message->stanzaid = strdup(stanzaid);const char* by = xmpp_stanza_get_attribute(stanzaidst, "by");same as earlier: trusting
byis under question.Same as previous — handled in #112
@@ -1442,0 +1434,4 @@if (message->plain == NULL) {message->plain = omemo_error_to_string(message->omemo_err);}} else if (message->plain != NULL) {this code is duplicated 3 times.
@@ -723,2 +708,4 @@return 0;}char*It should be
gchar*@@ -98,1 +72,3 @@free(resource);g_free(resource->name);g_free(resource->status);g_free(resource);we should set it to NULL for safety
it wasn't addressed. The pointer still is not set to
null, though with this function we can't actually do it, I guess. We need to pass pointer to pointer to actually do it. Instead, we can at least set it to null near each call toresource_destroy.Addressed in
60da899bdvia the caller-side route — see #1160 reply.@@ -112,6 +87,7 @@ _pendingPresence_free(ProfPendingPresence* presence)if (presence->last_activity)g_date_time_unref(presence->last_activity);free(presence->barejid);resource_destroy(presence->resource);We also clean it on line 138. Potential for double free. We need to set it to
NULLinresource.cfor safety. At least the resource itself.Fixed in
60da899bd— caller now nulls its pointer afterresource_destroy.I am not sure where to reply, but in the
roster_update_presence,resourceis a param. setting it tonullbefore returning does not increase safety. it will just override a local variable.Corrected
@@ -0,0 +1,4 @@[wrap-git]url = https://github.com/strophe/libstrophe.gitWe might want to point to our fork
Done — wrap now points at
devs/libstrophe-gh-mirror(live auto-mirror, 8h).1e372f6fa.@@ -0,0 +17,4 @@}voidcmd_ac_complete_filepath__segfaults_when_empty(void** state)segfaults_when_empty? Is it an actual behavior*? Or is it what we try to avoid? It's either a really strange test that expects undesired behavior or poor choice of a test name*Wouldn't it also result in a CI failure?
@@ -0,0 +78,4 @@assert_non_null(res3);assert_string_equal(res3, "/sendfile test_dir/file1.txt");// SHIFT-TABThis case doesn't test much :) Whether
taborshift+tab, we'll getfile2.txt. We need at least 3 files to make an actual test.@@ -330,0 +479,4 @@assert_string_equal("Could not convert \"23kk\" to a number.", err_msg);g_free(err_msg);err_msg = NULL;}We can add overflow and similar tests here.
@@ -0,0 +315,4 @@voidjid_is_valid_user_jid__is__true_for_valid_user_jid(void** state){assert_true(jid_is_valid_user_jid("myuser@mydomain/laptop"));We can add one more test for
myuser@mydomain/user@laptop. As per RFC6122, section 2.4,My feedback was partially addressed. I have updated the feedback to reflect current readiness.
Acceptance criteria:
alt+c) works in async without issues with nano, vimjabber.developer2 referenced this pull request2026-05-16 09:40:52 +00:00
97fd81b60btoadab585c09LGTM.
WIP: merge/upstream-fullto merge/upstream-fullin
editor.c, there is double#include "ui/ui.h"d3d7c328bLGTM
jabber.developer referenced this pull request2026-06-24 16:44:33 +00:00