Like the command-level CommandDescription.argspec, a verb's argspec is now
a nulstr, so that a verb may declare multiple alternative usage lines. The
verbs help table renders one synopsis line per nulstr entry in the
synopsis cell.
All verb argument specifications must gain an explicit NUL suffix. This
is a lot of churn, but I think this needs to be done. For now, there'll
be only one user of the new functionality (i.e. a verb with more than
one argspec), but I think we'll eventually want to use this in other
places too.
This converts systemd-homed, -hostnamed, -importd, -localed, -logind,
-machined, -networkd, -portebled, -resolved, -sysupdated, -timedated,
and -timesyncd.
The service name and description move from service_parse_argv() arguments
into a COMMAND() description that each daemon now defines. The
service_parse_argv() macro captures the calling binary's verb section,
where that COMMAND is found, while the options are still defined and
parsed in libshared, in the new "service" option namespace that each
COMMAND references. (The named namespace makes sure that options defined
by other translation units in libshared can never leak into the
services' parsers.)
The supported features are declared via option group filtering: each
COMMAND lists "Options" plus "Bus introspection" and/or "Runtime scope".
Unsupported options are now rejected by the parser as unknown options
(and are not shown in --help), replacing the runtime "This service does
not support …" errors.
The help output changes slightly: the "Bus introspection" and "Runtime
scope" option groups now get their own headings, and some descriptions
were reworded to the imperative form used elsewhere.
This time, various commands with non-trivial requirements are handled.
So the series is shorter, but has a pattern of feature additions and
then those new features being used. The way that --help output is
formatted is changed a bit in various places. Those changes are
described in individual commits.
22 'static int help()' functions left after this.
Almost all programs are simple and have only a single command. When
printing options, let's just use the first COMMAND block found by default.
This means we can drop the explicit program-name argument and reduce
copy&paste boilerplate.
SetTimezone() and SetLocalRTC() updated the Context before writing
/etc/localtime or /etc/adjtime. If the write failed, D-Bus properties
showed the new value and a repeated request with the same value returned
success without retrying the file update.
Keep the write helpers Context-based, but save the old value before
installing the requested state. Restore the old Context value when the
backing file update fails.
Add TEST-45 coverage for failed alternate localtime and adjtime paths.
Follow-up for: f401e48c2d
The .abstract field is always set, so it is fairly unambiguous to
recognize, and the value is usually long, so it's better to be brief.
It was only explicitly labelled in those two places.
Before this series, many help() functions opened the pager. This got lost
in the translation. Starting the pager requires access to arg_pager_flags.
While we could pipe this through in case when command_print_help() is
called directly, but this hard in the case when verb_help_auto() or
VERB_COMMON_HELP_AUTO/VERB_COMMON_HELP_AUTO_HIDDEN are used. Let's instead
extend the (const static) CommandDescription structure with a pointer to
the arg_pager_flags variable. This restores the pager functionality in a
uniform fashion with a one-line change in the individual binaries.
bus_message_print_all_properties() peeks the variant type, but then the
print callback and the default bus_print_property() each peek the value
type again, so there can be up to two redundant calls per property. Peek
it once up front and pass it through.
With this, in my tests `systemctl show` over 160 units decreases in
instructions retired from 1167.6M to 1155.7M, so about 1%.
Previously, when a library was dynamically loaded via a helper function
inside libsystemd-shared.so, the resulting dlopen ELF note was not
propagated to the invoking executable's ELF metadata.
This commit explicitly annotates each executable with the relevant
dlopen ELF notes for any optional dependencies it might potentially
load. This ensures that package managers and build systems can properly
discover runtime dependencies that are triggered indirectly.
The new polkit will return a new detail regarding a successful
authentication: the actual result type, which we can use to
see whether the user authenticated as admin. This can be used
to grant additional privileges.
I am really not a fan of full code lines passed to macros as parameters.
Let's get rid of the 3rd parameter of FOREACH_OPTION() hence:
1. Let's return errors just as a regular value (though a negative one),
that can be handled via a OPTION_ERROR case statement for the switch.
This normalizes handling of the error, just like any other event
returned by the option parser.
2. In order to avoid exploding the amount of boilerplate in each use
(that just propagates the error on OPTION_ERROR), let's then
introduce an explicit FOREACH_OPTION_OR_RETURN(), that returns from
the calling function on its own (and makes that clear in the name).
Together this cleans up, normalizes the logic and shortens the code.
It was requested to make the 'c', 'opt', and 'arg' params the same, i.e.
defined through the FOREACH_OPTION macro. But we can't do that easily,
because 'c' was defined in the for loop definition, and we can only
define variables of the same type in that way. Also, in some cases we
need only 'c', in other cases with need 'c' and 'arg, in some cases 'c'
and 'opt', and in other cases all three. We'd need to either
conditionalize or mark those variables with _unused_ to deal with
compiler warnings. But a different approach works quite nicely: add
state.opt and state.arg to show the current option and it's argument.
(The short names are picked on purpose to reduce verbosity since those
are used a lot.)
gcc and newer clang seem to be fine with it, but clang 14, 16, 18
is unhappy:
../src/timedate/timedatectl.c:1006:25: error: fallthrough annotation does not directly precede switch label
_fallthrough_;
^
_fallthrough_ doesn't seem to be used very often in option parsing,
so let's remove the use for now.
--help is identical, except for alignment of second column and
changed strings in the standarized options.
Co-developed-by: Claude <claude@anthropic.com>
We generally want to print an error message if table_print()
fails. Add a helper function for this and use it consistently.
This does one of the three things depending on the call site:
- a no-change reformatting of the code
- change from a custom message to the generic one
- addition of the error message where previously none was printed
In the third case, the actual use impact is very small, since the
table formatting is very unlikely to fail. But if it did, we would
often return an error without any message whatsoever, which we
never want to do.
Almost all callers of table_print() specify stdout or NULL (equivalent
to stdout) as the output stream. Simplify things by not requiring the
the stream to be specified.
In almost all cases, the printing of the table is surrounded by normal
printfs() that don't do explicit flushing and for which we don't check
the output stream status. Let's simplify most callers and skip this
step. The reason is not so much to avoid the extra step itself, but
instead to avoid the _handling_ of the potential failure. We generally
only want to print an error message for ENOMEM and other "internal"
errors, so strictly speaking we should filter out the errors from the
stream. By skipping the flush&check step we implicitly do this.
We often have a pattern where the same verb function is used for
multiple actions. This leads to an antipattern where we figure out what
action needs to be taken from argv[0] multiple times: often once in
parse_argv() to figure out what options are allowed, then once again
implicitly in dispatch_verb(), and then again in the action verb itself.
Let's allow passing a parameter into the verb to simplify this.
This matches a pattern we have in conf-parser.h, where we have both
void *userdata (more global) and void *data (per-config item). Here,
I opted for uintptr_t userdata. It seems that most of the time we'll
want to just pass an enum value. This works OK with no casts. I also
tried a void* and union. In both cases, much more boilerplate is needed:
either a cast or a macro to help avoid compiler warnings. uintptr_t
seems generic enough to cover foreseeable usecases with no fuss.
This is a noop refactoring. See next commit for an example.
Let's not leak details from src/shared and src/libsystemd into
src/basic, even though you can't actually do anything useful with
just forward declarations from src/shared.
The sd-forward.h header is put in src/libsystemd/sd-common as we
don't have a directory for shared internal headers for libsystemd
yet.
Let's also rename forward.h to basic-forward.h to keep things
self-explanatory.
glibc (and also musl, though we do not officially support it yet)
silently ignores colon prefix in $TZ. Let's always not prefix the
timezone.
tzset(3) states:
> A nonempty value of TZ can be one of two formats, either of which can
> be preceded by a colon which is ignored.
Addresses https://github.com/systemd/systemd/pull/38876#discussion_r2384347594.
Some configuration files that need updates are directly under in /etc. To
update them atomically, we need write access to /etc. For Ubuntu Core this is
an issue as /etc is not writable. Only a selection of subdirectories can be
writable. The general solution is symlinks or bind mounts to writable places.
But for atomic writes in /etc, that does not work. So Ubuntu has had a patch
for that that did not age well.
Instead we would like to introduce some environment variables for alternate
paths.
* SYSTEMD_ETC_HOSTNAME: /etc/hostname
* SYSTEMD_ETC_MACHINE_INFO: /etc/machine-info
* SYSTEMD_ETC_LOCALTIME: /etc/localtime
* SYSTEMD_ETC_LOCALE_CONF: /etc/locale.conf
* SYSTEMD_ETC_VCONSOLE_CONF: /etc/vconsole.conf
* SYSTEMD_ETC_ADJTIME: /etc/adjtime
While it is for now expected that there is a symlink from the standard, we
still try to read them from that alternate path. This is important for
`/etc/localtime`, which is a symlink, so we cannot have an indirect symlink or
bind mount for it.
Since machine-id is typically written only once and not updated. This commit
does not cover it. An initrd can properly create it and bind mount it.
constants.h defines a bunch of constants that use macros from
time-util.h, so add the needed include. To avoid cyclic dependencies,
we move the HAS_FEATURE_MEMORY_SANITIZER and HAS_FEATURE_ADDRESS_SANITIZER
logic to macro.h and get rid of the constants.h include there.
The rest of this commit is adding missing constants.h includes where
needed due to it being removed from macro.h.
Let's move some logic from _DEFINE_MAIN_FUNCTION() and other places
in main-func.h into functions that we implement in main-func.c to
allow moving some included headers from the header to the .c file.
Currently, NOTIFY_READY from daemon-util.h conflicts with NOTIFY_READY
from NotifyState from service.h so let's rename the constants to avoid
the conflict.
Additionally, using the log context makes sure the extra fields are
applied to all log messages generated while the context is in place,
rather than only log messages logged with log_unit_xxx() in timedated
itself.
It also means the unit name is prefixed to all log messages logged
within that context. While it's not clear whether we always want the
unit name to be attached to library log messages, we also don't want
the unit name to not be attached to any library log messages, so we opt
to add more rather than less information by adding the unit name everywhere
for now.
In the future we can look into some log.h helpers to enable/disable
adding the prefix to the following log messages or not.
Now that the necessary functions from log.h have been moved to macro.h,
we can stop including log.h in macro.h. This requires modifying source
files all over the tree to include log.h instead.
This introduce LOG_ITEM() macro that checks arbitrary formats in
log_struct().
Then, drop _printf_ attribute from log_struct_internal(), as it does not
help so much, and compiler checked only the first format string.
Hopefully, this silences false-positive warnings by Coverity.
Zbigniew Jędrzejewski-Szmek
felix h
dongshengyuan
Chris Down
Yu Watanabe
Luca Boccassi
Lennart Poettering
Mike Yuan
Daan De Meyer
Lennart Poettering
Valentin David
Nick Rosbrook