Cleanup of the conversion-safety warnings exposed by enabling
-Wconversion / -Wsign-compare in the previous commit, plus guard
clauses at the few places where unsigned arithmetic could actually
misbehave.
Build:
- configure.ac drops -Wno-error=conversion and
-Wno-error=float-conversion. Only -Wno-error=sign-conversion and
-Wno-error=sign-compare remain, gating the ~230 sign warnings
inherited from upstream that will be cleaned up in follow-ups.
Type / conversion fixes (no behaviour change):
- Length-like locals in command/cmd_ac.c, command/cmd_funcs.c,
pgp/gpg.c, tools/autocomplete.c, tools/parser.c and ui/mucwin.c
switched from int to size_t / glong (matching strlen /
g_utf8_strlen return type) so we no longer need an (int) cast and
loop counters / array sizes stay in their natural unsigned domain.
- g_timer_elapsed / GTimeSpan -> int casts in session.c, iq.c,
core.c, server_events.c, window.c.
- _win_print_wrapped: indent parameter and local curx/maxx switched
from size_t to int to match _win_indent / getcurx / getmaxx.
- Port casts (int -> unsigned short) at the libstrophe boundary in
connection.c and session.c, each preceded by
g_assert(port >= 0 && port <= UINT16_MAX) so the truncation is
documented at the call-site.
- curl_off_t / fread size_t results cast at usage in http_upload.c,
http_download.c, omemo/crypto.c.
- strtoul results cast to uint32_t in xmpp/omemo.c and omemo/omemo.c
where device/prekey IDs are genuinely 32-bit.
- config/color.c: fg/bg/palette indices switched to `short`
end-to-end (find_col, color_hash, find_closest_col,
_color_pair_cache_get, cache.pairs), so the ncurses init_pair
boundary needs at most one (short)i cast for the cache index. Also
TODO-noted: init_extended_pair is needed for >15-bit palettes.
- xmpp/avatar.c: float arithmetic explicitly casts its int operands.
- tests/functionaltests/proftest.c: read() result handling uses
size_t for the accumulator, _read_output returns ssize_t, and the
buffer-shift check happens before space subtraction so the
expression cannot underflow.
Real-risk guard clauses (the part that actually fixes bugs):
- src/ui/statusbar.c _tabs_width: `end > opened_tabs - 1` rewritten
as `end < opened_tabs` so opened_tabs == 0 no longer underflows.
- src/ui/statusbar.c _status_bar_draw_extended_tabs: the mirror
comparison rewritten as `end >= opened_tabs`.
- src/ui/statusbar.c status_bar_draw: replaced
`MAX(0, getmaxx - (int)_tabs_width)` with an explicit precheck
before subtraction.
- src/omemo/omemo.c prekey selection: prekey_index is now uint32_t
and randomized into an unsigned buffer, so modulo with prekeys_len
cannot yield a negative index for g_list_nth_data.
- src/omemo/crypto.c omemo_decrypt_func: PKCS#5/PKCS#7 unpadding
reads `plaintext[plaintext_len - 1]`, which would underflow on a
malformed empty ciphertext and read past the heap buffer. Reject
plaintext_len == 0 before the padding peek and validate the
padding byte against the buffer length before the unpad loop.
Initialise plaintext = NULL so the early `goto out` cannot free
uninitialised memory.
- src/ui/inputwin.c (4 mbrlen sites) and src/ui/window.c
_win_print_wrapped: mbrlen() returns 0 for the null wide
character. The existing checks rejected (size_t)-1 / -2 but
treated 0 as a valid step, so the surrounding loops would either
advance by SIZE_MAX (i += ch_len - 1) or spin in place
(word_pos += 0 forever). Add `|| ch_len == 0` to each guard;
inside the spell-check word-emission loop also fall back to a
one-byte advance.
- Defensive `len > 0 ? len - 1 : 0` prechecks at the strlen-based
g_strndup / loop sites in ui/console.c, plugins/c_api.c and
plugins/python_plugins.c.
Turn on the stricter conversion-safety warnings and wire in an optional
sanitizer build mode. Nothing is promoted to a hard error yet: the
warnings become part of the build output and can be cleaned up
incrementally without blocking development.
- Drop -Wno-sign-compare and add -Wsign-compare + -Wconversion to the
GCC-specific warnings loop (probed for compiler support).
- Add --enable-sanitizers flag that enables ASan + UBSan +
-fsanitize=unsigned-integer-overflow with -fno-sanitize-recover=all.
Off by default; intended for a dedicated CI job.
- Under PACKAGE_STATUS=development (-Werror), opt out of turning the
new conversion/sign-compare warnings into hard errors via
-Wno-error=sign-conversion/-conversion/-float-conversion/-sign-compare.
Existing codebase has ~240 sign-conversion, ~87 other narrowing and a
handful of sign-compare warnings (mostly inherited from upstream);
they stay visible but don't block development builds.
- Route glib/gio CFLAGS through -isystem so macros like
GPOINTER_TO_UINT in glibconfig.h don't generate noise.
Split functional tests into 4 parallel groups and add check-functional-parallel target (~3x faster CI runs).
Add branch-aware LCOV coverage reporting with new --enable-coverage option and lcov summary in CI pipeline.
Enable ccache via -C configure flag for faster recompilations.
Install lcov in all Docker images and use --depth 1 git clones + parallel make -j$(nproc) for quicker container builds.
Update CONTRIBUTING.md with instructions for parallel test groups and adding new ones.
All changes are tightly related CI/performance improvements developed in sequence. No external service uploads (e.g. Codecov skipped due to Gitea incompatibility).
* Also pass `$*` to `configure` when invoking `ci-build.sh`, so one can
e.g. run `./ci-build.sh --without-xscreensaver`
Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
This commit fixes few issues related to OMEMO autodetection:
1. Absence of libsignal-protocol-c doesn't turn OMEMO off, just prints a
notice message. It should (a) set BUILD_OMEMO=false and (b) terminate
with an error on --enable-omemo=yes.
2. Check for gcrypt continues even if libsignal-protocol-c fails. In this
case, LIBS variable can be updated with unneeded -lgcrypt.
3. Similarly to item 2., if libsignal-protocol-c is present, but gcrypt
isn't, variable LIBS is updated with unneeded library.
To resolve the above issues, use intermediate variable OMEMO_LIBS to
accumulate required libraries and set BUILD_OMEMO only when all checks
are passed.
In 28a9605a1 we migrated from AC_CHECK_LIB which defines HAVE_LIBXSS automatically. With pkg-config way you need it explicit. And also x11 is needed or else linking will fail missing XFree().
Patch provided by Paul Fertser and comitted by jubalh.
Thanks Paul!
Fix https://github.com/profanity-im/profanity/issues/1695
Before we got an error when libqrencode was not installed:
`src/ui/console.c:52:10: fatal error: qrencode.h: No such file or
directory`
Which looked like HAVE_QRENCODE was true.
config.status showed:
`config.status:D["HAVE_QRENCODE"]=" 0"`
Holger pointed to me that there is not just true and false but defined
and undefined.
So one solution was to use `#if HAVE_QRENCODE == 1` to check for the
actual value.
Or dont define HAVE_QRENCODE in the non present case at all.
In all the other HAVE_ variables we use this approach.
I think i at first chose the wrong way out of confusion with BUILD_ and
HAVE_.
Option -ggdb3 (same as -g3) doesn't affect code-generation, just adds
to the binary size of an executable. Automake strips installed
binaries automatically when "make install-strip" is run so enabling
this option permanently doesn't affect the end result while allowing
for hassle-free interactive debugging.
If one is running a stripped binary it's still possible to attach GDB
by process PID and then use "file /path/to/unstripped/profanity" to
get all symbols and lines data.
Autoconf can pre-populate this variable with essential parameters so
it should be appended to rather than overridden.
While at it, don't miss to append CFLAGS for libstrope which is needed
if it's installed to a non-default location.
AM_INIT_AUTOMAKE requires AC_ARG_PROGRAM so should go after
AC_CANONICAL_TARGET for proper program name mangling.
This fixes "AC_ARG_PROGRAM was called before AC_CANONICAL_TARGET"
warning as emitted by autoconf 2.69.
Fixing build failure detected by sr.ht for OpenBSD:
```
src/ui/inputwin.c: In function '_inp_rl_startup_hook':
src/ui/inputwin.c:444:5: error: implicit declaration of function 'rl_bind_keyseq'; did you mean 'rl_bind_key'? [-Werror=implicit-function-declaration]
444 | rl_bind_keyseq("\\e1", _inp_rl_win_1_handler);
| ^~~~~~~~~~~~~~
| rl_bind_key
cc1: all warnings being treated as errors
```
Seems like both OSX and OpenBSD need CPPFLAGS here.
Previously it relied on AX_PYTHON_DEVEL, which in turn executes
python-config to get the build flags. However this does not work while
cross compiling because we can't execute the python-config build for the
target platform. To circumvent this problem the python build flags are
now queried via pkgconfig, which has the drawback of not having some
extra build flags, but they do not seem to be needed.
I tested this patch with the termux build system and it build without
their existing hack of injecting python after the configure step. I also
tested non cross compile build on Arch Linux and it also still works.
Fixes#851
Remove support for libsignal-protocol-c < 2.3.2.
Debian 10 uses 2.3.2, Debian 11 and 12 use 2.3.3.
openSUSE from 15.2 onward uses 2.3.3.
Fedora since 28 uses 2.3.2.
We should be good.
