Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some more fixes & improvements #2023

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Some more fixes & improvements #2023

wants to merge 13 commits into from

Conversation

sjaeckel
Copy link
Member

@sjaeckel sjaeckel commented Mar 7, 2025

Check commit messages for details.

@sjaeckel sjaeckel requested a review from jubalh March 7, 2025 11:02
sjaeckel added 2 commits March 7, 2025 21:09
This allows to kind of automate what profanity should do as first jobs,
e.g. `--cmd /foo --cmd /bar --cmd /quit` so one can easily check for memory
leaks.

Signed-off-by: Steffen Jaeckel <[email protected]>
Instead of adding stuff to `_shutdown()`, we can now register a shutdown
routine from the respective `init()` function of our modules.

This also has the advantage, that we're sure they're called in reverse
order from how the initialization happened.

I didn't simply use `atexit()` because POSIX says that the number of
possible callbacks is limited (min 32) and I was not sure whether
we will maybe extend this number at one point.

Signed-off-by: Steffen Jaeckel <[email protected]>
@sjaeckel sjaeckel force-pushed the debug-1946-1994 branch 3 times, most recently from 9d745d0 to 6f74a98 Compare March 7, 2025 21:35
@sjaeckel sjaeckel requested a review from jubalh March 7, 2025 21:36
@sjaeckel sjaeckel force-pushed the debug-1946-1994 branch 3 times, most recently from 7de8243 to 6ea5e50 Compare March 7, 2025 22:09
I hope I didn't overshoot :)

Signed-off-by: Steffen Jaeckel <[email protected]>
`ProfMessage` was not completely initialized, fix this by using `calloc()`
for the allocation.

Signed-off-by: Steffen Jaeckel <[email protected]>
* destroy/free/shutdown/close in reverse order of allocation
* more static/auto_Xfree
* less variables/strlen/GString
* properly `\0`-terminate string of testcase

Signed-off-by: Steffen Jaeckel <[email protected]>
While trying to get the unit tests working again I stumbled over all those
things that I thought could be better^TM.

Now we also know "TODO: why does this make the test fail?" - because
the unit tests are brittle AF ... and we have to init the subsystems
we use in the test, otherwise the cleanup will fail...

BTW. you can now also only run a single test ... or a pattern or so ...
you'd have to read how `cmocka_set_test_filter()` works exactly.

Signed-off-by: Steffen Jaeckel <[email protected]>
@sjaeckel sjaeckel force-pushed the debug-1946-1994 branch 3 times, most recently from 1f0756b to d1aa295 Compare March 10, 2025 14:47
@sjaeckel sjaeckel requested a review from jubalh March 10, 2025 15:09
* 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 <[email protected]>
@jubalh
Copy link
Member

jubalh commented Mar 11, 2025

Need to take a closer look at ba50497 otherwise already LGTM

* Less leaks
* Less allocations

Signed-off-by: Steffen Jaeckel <[email protected]>
Instead of always returning a `strdup()`'ed version, maybe return NULL and
handle this in the calling code.

This is still not true for `plugins_pre_chat_message_display()`, where we
return the passed `messag` in case there are no plugins, since this would
require a bigger refactor of more parts.

This also
* merges the implementation of `sv_ev_incoming_private_message()` and
  `sv_ev_delayed_private_message()`, since they differed only by a single
  line.
* untangles the `#ifdef` mess in `cl_ev_send_muc_msg_corrected()`.

Signed-off-by: Steffen Jaeckel <[email protected]>
Signed-off-by: Steffen Jaeckel <[email protected]>
* Add an internal `_notifier_uninit()` function.
* Instead of `#ifdef` inside `notify()`, provide different target-specific
static implementations.

This also introduces `log_error_once()`.

Signed-off-by: Steffen Jaeckel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants