We had yet-another table of descriptive strings to use in error messages.
I started thinking how to synchronize them with the strings in logind, but
ultimately I think it's better to remove those altogether. Those strings
should almost never be used: normally if the call fails, logind will provide
an error message itself, which is probably more detailed than what we can
figure out on the client side. And the most important part that we want to
show here is what exactly we called, in particular RebootWithFlags vs. Reboot,
etc. By using the "descriptive strings" we were obfuscating this. So let's just
simplify our code and print the actual method name, since this is more useful
as an error statement that is googlable and unique.
While at it, let's print the correct method name ;)
oss-fuzz reports timeouts which are created by appending to a very long strv.
The code is indeed not very efficient, but it's designed for normal
command-line use, where we don't expect more than a dozen of entries. The fact
that it is slow with ~100k entries is not particularly interesting.
In the future we could rework the code to have better algorithmic complexity.
But let's at least stop oss-fuzz from wasting more time on such examples.
(My first approach was to set max_len in .options, but apparently this doesn't
work for hongfuzz and and AFL.)
oss-fuzz-34527: https://oss-fuzz.com/issue/5722283944574976
We have vendor presets, and local admin presets, and runtime presets
(under /usr/lib, /usr/local/lib and /etc, /run, respectively). When we
display preset state, it can be configured in any of those places, so
we shouldn't say anything about the origin.
(Another nice advantage is that it improves alignment:
[root@f36 ~]# systemctl list-unit-files multipathd.service
UNIT FILE STATE VENDOR PRESET
multipathd.service enabled enabled
^ this looks we have a "PRESET" column that is empty.)
"enabled" state is highlighted in green and "disabled" state is
highlighted in yellow because I felt that white and grey colors were not
so distinguishable. Other states are not highlighted. Any other coloring
suggestions are welcome!
Closes#16932.
Some calls to lookup_path_init() were not followed by any log emission.
E.g.:
$ SYSTEMD_LOG_LEVEL=debug systemctl --root=/missing enable unit; echo $?
1
Let's add a helper function and use it in various places.
$ SYSTEMD_LOG_LEVEL=debug build/systemctl --root=/missing enable unit; echo $?
Failed to initialize unit search paths for root directory /missing: No such file or directory
1
$ SYSTEMCTL_SKIP_SYSV=1 build/systemctl --root=/missing enable unit; echo $?
Failed to initialize unit search paths for root directory /missing: No such file or directory
Failed to enable: No such file or directory.
1
The repeated error in the second case is not very nice, but this is a niche
case and I don't think it's worth the trouble to trying to avoid it.
This normalizes naming of functions operating on BootConfig objects.
Let's always call them boot_config_xyz(), like our usual way to name
stuff.
moreover, move the BootConfig parameter to the beginning, as it's not a
return value (which we typically move to the end of the parameter list),
but simply an object, that even happens to be initialized already.
With these changes the functions are more like our usual way to call
things, and less surprises are good.
Same idea as 03677889f0.
No functional change intended. The type of the iterator is generally changed to
be 'const char*' instead of 'char*'. Despite the type commonly used, modifying
the string was not allowed.
I adjusted the naming of some short variables for clarity and reduced the scope
of some variable declarations in code that was being touched anyway.
We should treat ./some.service and $PWD/some.service as equivalent. But we'd
try to send the relative paths over dbus, which can't work well:
$ sudo systemctl enable ./test2.service
Failed to look up unit file state: Invalid argument
$ sudo systemctl enable $PWD/test2.service
Created symlink /etc/systemd/system/multi-user.target.wants/test2.service → /home/zbyszek/src/systemd/test2.service.
Created symlink /etc/systemd/system/test2.service → /home/zbyszek/src/systemd/test2.service.
Now both are equivalent.
The fall-through to reboot doesn't seem to make sense. It won't happen
in the current code. Filtering the actions on client side is not needed
either as the server will refuse unsupported operations anyway.
Let's split out the inner parts of verb_daemon_reload() as a function
daemon_reload() and then stop using the former outside of the verbs
logic, and instead call the latter whenever we need to reload the daemon
as auxiliary opeation.
This should make our logic more systematic as we don't have to provide
fake or misleading argc/argv to verb_daemon_reload() anymore.
Let's clean up our function naming a bit, and always name the
verb_xyz(), where the xyz maps to the command line verb as closely as
possible.
No actual code changes, just an attempt to make the systemctl sources a
bit more systematic, and less surprising.
Currently the `--timestamp=` option has no effect on timestamps shown by
`systemctl show`, let's fix that.
Spotted in #22567.
Before:
```
$ systemctl show --timestamp=us+utc systemd-journald | grep Timestamp=
ExecMainStartTimestamp=Sat 2021-12-11 15:25:57 CET
StateChangeTimestamp=Sat 2021-12-11 15:25:57 CET
InactiveExitTimestamp=Sat 2021-12-11 15:25:57 CET
ActiveEnterTimestamp=Sat 2021-12-11 15:25:57 CET
ActiveExitTimestamp=Sat 2021-12-11 15:25:57 CET
InactiveEnterTimestamp=Sat 2021-12-11 15:25:57 CET
ConditionTimestamp=Sat 2021-12-11 15:25:57 CET
AssertTimestamp=Sat 2021-12-11 15:25:57 CET
```
After:
```
$ systemctl show --timestamp=us+utc systemd-journald | grep Timestamp=
ExecMainStartTimestamp=Sat 2021-12-11 14:25:57.177848 UTC
StateChangeTimestamp=Sat 2021-12-11 14:25:57.196714 UTC
InactiveExitTimestamp=Sat 2021-12-11 14:25:57.177871 UTC
ActiveEnterTimestamp=Sat 2021-12-11 14:25:57.196714 UTC
ActiveExitTimestamp=Sat 2021-12-11 14:25:57.144677 UTC
InactiveEnterTimestamp=Sat 2021-12-11 14:25:57.176331 UTC
ConditionTimestamp=Sat 2021-12-11 14:25:57.176980 UTC
AssertTimestamp=Sat 2021-12-11 14:25:57.176980 UTC
```
It's totally not OK to write to the strings returned by it, the data is
shared by all code that references the message.
While we are at it, simplify the code via
json_variant_set_field_string().
Follow-up for: 5ef599b324
For shutdowns don't fall back to starting the target directly if talking
to logind failed with auth failure. That would just lead to another
polkit auth attempt.
The code at this point is not able to tell whether it was called as
halt/poweroff/reboot or shutdown with time "now".
The code also takes a shortcut to skip logind if called as root.
That however means asking shutdown for immediate action won't trigger a
wall message.
As per https://github.com/systemd/systemd/issues/8424#issuecomment-374677315
all commands should trigger a wall message.
That simplifies the code as we can try logind first always.
This commit adds a function which converts a bus message containing the
environment variables to a JSON object and uses this function to support
JSON formatted output for the "systemctl show-environment" command.
Fixes#21348
David Tardon
Luca Boccassi
Yu Watanabe
Zbigniew Jędrzejewski-Szmek
Elias Probst
amarjargal
Lennart Poettering
Ludwig Nussel
Daan De Meyer
Frantisek Sumsal
Jan Janssen
Joris Hartog