[systemd-devel] strna/strempty/strnull around job_*_to_string()

[Lukasz Stelmach]

Hi.

For a reason I cannot determine (nor, probably, deal with directly)
gcc I have to use started to issue warnings (errors in fact due tu
-Werror) about an argument for '%s'[1] being null.

--8<---------------cut here---------------start------------->8---
In file included from ../src/core/dbus-job.h:7,
                 from ../src/core/job.c:11:
../src/core/job.c: In function 'job_finish_and_invalidate':
../src/core/job.c:976:27: error: '%s' directive argument is null [-Werror=format-overflow=]
  976 |         log_unit_debug(u, "Job %" PRIu32 " %s/%s finished, result=%s", j->id, u->id, job_type_to_string(t), job_result_to_string(result));
      |                           ^~~~~~~
../src/core/unit.h:878:190: note: in definition of macro 'log_unit_full'
  878 | u->manager->unit_log_field, _u->id, _u->manager->invocation_log_field, _u->invocation_id_string, ##__VA_ARGS__) : \
      |                                                                                                    ^~~~~~~~~~~

../src/core/job.c:976:9: note: in expansion of macro 'log_unit_debug'
  976 |         log_unit_debug(u, "Job %" PRIu32 " %s/%s finished, result=%s", j->id, u->id, job_type_to_string(t), job_result_to_string(result));
      |         ^~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--8<---------------cut here---------------end--------------->8---

I've fixed it by adding strna() around the two *_to_string() helpers.

Funnily enough this doesn't happen anywhere else in the code and is
associated with -O2 switch. Even more so, adding strna() only around
job_result_to_string() is enough to silence the warnings.

I grepped for other calls to *_to_string() helpers and unsurprisingly
there is a ton of them. I didn't mange to figure out if returning
something different than NULL would be safe in all possible contexts. I
assume it wouldn't. Thus the invocations need to be inspected one by one
and wrapped in str*() widgets.

Because I'd like to send you my fix to be kept upstream but I can't fix
all _to_strings() in every module. However, reviewing two dozen of them
in src/core/job.c and fixing those that are arguments to format strings
isn't a problem at all. The question is:

Which str*() in which context should I use?

There are only three different helpers used in job.c

    - job_type_to_string() (18 occurences)
    - job_result_to_string() (6)
    - job_state_to_string() (2)


Most of them as an argument of a format string for
log_unit_{debug,warning}() and for four invocations of
log_unit_struct(). I guess, the latter with format strings like

  "JOB_TYPE=%s"

should use something different than human readable messages in the
former. Any hints, suggestions?


[1] https://github.com/systemd/systemd/blob/1c53c3bab168c03874f328d62719a2e1f3a703e8/src/core/job.c#L996 
    the line numbers are off because I build an older version, but as
    far as I can tell nothing has changed in the *_to_string generator
    macro.

Kind regards,
-- 
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 487 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/systemd-devel/attachments/20230628/507ff7f3/attachment.sig>