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
Attachment:
signature.asc
Description: PGP signature
