There are two ways in swich sd_login_* functions acquire data:
some are derived from the cgroup path, but others use the data serialized
by logind.
When the tests are executed under Fedora's mock, without systemd-spawn
but instead in a traditional chroot, test-login gets confused:
the "outside" cgroup path is visible, so sd_pid_get_unit() and
sd_pid_get_session() work, but sd_session_is_active() and other functions
that need logind data fail.
Such a buildroot setup is fairly bad, but it can be encountered in the wild, so
let's just skip the tests in that case.
/* Information printed is from the live system */
sd_pid_get_unit(0, …) → "session-237.scope"
sd_pid_get_user_unit(0, …) → "n/a"
sd_pid_get_slice(0, …) → "user-1000.slice"
sd_pid_get_session(0, …) → "237"
sd_pid_get_owner_uid(0, …) → 1000
sd_pid_get_cgroup(0, …) → "/user.slice/user-1000.slice/session-237.scope"
sd_uid_get_display(1000, …) → "(null)"
sd_uid_get_sessions(1000, …) → [0] ""
sd_uid_get_seats(1000, …) → [0] ""
Assertion 'r >= 0' failed at src/libsystemd/sd-login/test-login.c:104, function test_login(). Aborting.
Let's clean up hostname_is_valid() a bit: let's turn the second boolean
argument into a more explanatory flags field, and add a flag that
accepts the special name ".host" as valid. This is useful for the
container logic, where the special hostname ".host" refers to the "root
container", i.e. the host system itself, and can be specified at various
places.
let's also get rid of machine_name_is_valid(). It was just an alias,
which is confusing and even more so now that we have the flags param.
Ubuntu builds on the Launchpad infrastructure run inside a chroot that does
not have the sysfs cgroup dirs mounted, so this call will return ENOMEDIUM
from cg_unified_cached() during the build-time testing, for example when
building the package in a Launchpad PPA.
A long time some function only worked when in a session, and the test
didn't execute them when sd_pid_get_session() failed. Let's always call
them to increase coverage.
While at it, let's test for ==0 not >=0 where we don't expect the function
to return anything except 0 or error.
sd_seat_get_sessions() would return 0 in the 'n_uids' (now 'ret_n_uids') output
parameter when 'uid' (now 'ret_uids') was passed as NULL.
While at it, drop FOREACH_WORD() use.
Also use any whitespace as separator. In practice this shouldn't matter, since
logind always uses spaces, but it seems nicer to not specify this explicitly,
and the default is more flexible.
We don't need a seperate output parameter that is of type int. glibc() says
that the type is "unsigned", but the kernel thinks it's "int". And the
"alternative names" interface also uses ints. So let's standarize on ints,
since it's clearly not realisitic to have interface numbers in the upper half
of unsigned int range.
Now that we don't (mis-)use the env file parser to parse kernel command
lines there's no need anymore to override the used newline character
set. Let's hence drop the argument and just "\n\r" always. This nicely
simplifies our code.
These lines are generally out-of-date, incomplete and unnecessary. With
SPDX and git repository much more accurate and fine grained information
about licensing and authorship is available, hence let's drop the
per-file copyright notice. Of course, removing copyright lines of others
is problematic, hence this commit only removes my own lines and leaves
all others untouched. It might be nicer if sooner or later those could
go away too, making git the only and accurate source of authorship
information.
This part of the copyright blurb stems from the GPL use recommendations:
https://www.gnu.org/licenses/gpl-howto.en.html
The concept appears to originate in times where version control was per
file, instead of per tree, and was a way to glue the files together.
Ultimately, we nowadays don't live in that world anymore, and this
information is entirely useless anyway, as people are very welcome to
copy these files into any projects they like, and they shouldn't have to
change bits that are part of our copyright header for that.
hence, let's just get rid of this old cruft, and shorten our codebase a
bit.
Most our other parsing functions do this, let's do this here too,
internally we accept that anyway. Also, the closely related
load_env_file() and load_env_file_pairs() also do this, so let's be
systematic.
Previously we were a bit sloppy with the index and size types of arrays,
we'd regularly use unsigned. While I don't think this ever resulted in
real issues I think we should be more careful there and follow a
stricter regime: unless there's a strong reason not to use size_t for
array sizes and indexes, size_t it should be. Any allocations we do
ultimately will use size_t anyway, and converting forth and back between
unsigned and size_t will always be a source of problems.
Note that on 32bit machines "unsigned" and "size_t" are equivalent, and
on 64bit machines our arrays shouldn't grow that large anyway, and if
they do we have a problem, however that kind of overly large allocation
we have protections for usually, but for overflows we do not have that
so much, hence let's add it.
So yeah, it's a story of the current code being already "good enough",
but I think some extra type hygiene is better.
This patch tries to be comprehensive, but it probably isn't and I missed
a few cases. But I guess we can cover that later as we notice it. Among
smaller fixes, this changes:
1. strv_length()' return type becomes size_t
2. the unit file changes array size becomes size_t
3. DNS answer and query array sizes become size_t
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=76745
Files which are installed as-is (any .service and other unit files, .conf
files, .policy files, etc), are left as is. My assumption is that SPDX
identifiers are not yet that well known, so it's better to retain the
extended header to avoid any doubt.
I also kept any copyright lines. We can probably remove them, but it'd nice to
obtain explicit acks from all involved authors before doing that.
This macro will read a pointer of any type, return it, and set the
pointer to NULL. This is useful as an explicit concept of passing
ownership of a memory area between pointers.
This takes inspiration from Rust:
https://doc.rust-lang.org/std/option/enum.Option.html#method.take
and was suggested by Alan Jenkins (@sourcejedi).
It drops ~160 lines of code from our codebase, which makes me like it.
Also, I think it clarifies passing of ownership, and thus helps
readability a bit (at least for the initiated who know the new macro)
This is useful so that callers know whether anything at all and how much
was flushed.
This patches through users of this functions to ensure that the return
values > 0 which may be returned now are not propagated in public APIs.
Also, users that ignore the return value are changed to do so explicitly
now.
Observed when running from the console of a systemd nspawn container
(see failure below).
The value of r was tested, when r was last set by
sd_session_can_graphical(). This did not correspond to the value expected.
Fix the code, so we compare relevant values now. Hopefully :).
Test failure
------------
/* Information printed is from the live system */
sd_pid_get_unit(0, …) → "session-13.scope"
sd_pid_get_user_unit(0, …) → "n/a"
sd_pid_get_slice(0, …) → "user-1000.slice"
sd_pid_get_session(0, …) → "13"
sd_pid_get_owner_uid(0, …) → 1000
sd_pid_get_cgroup(0, …) → "/user.slice/user-1000.slice/session-13.scope"
sd_uid_get_display(1000, …) → "13"
sd_uid_get_sessions(1000, …) → [2] "15 13"
sd_uid_get_seats(1000, …) → [1] "seat0"
sd_session_is_active("13") → yes
sd_session_is_remote("13") → no
sd_session_get_state("13") → "active"
sd_session_get_uid("13") → 1000
sd_session_get_type("13") → "tty"
sd_session_get_class("13") → "user"
sd_session_get_display("13") → "n/a"
sd_session_get_remote_user("13") → "n/a"
sd_session_get_remote_host("13") → "n/a"
sd_session_get_seat("13") → "seat0"
sd_session_can_multi_seat("seat0") → no
sd_session_can_tty("seat0") → no
sd_session_can_graphical("seat0") → no
sd_uid_get_state(1000, …) → active
Assertion '!!k == !!r' failed at ../src/libsystemd/sd-login/test-login.c:191, function test_login(). Aborting.
Other functions in sd-login generally allow the output parameter to be NULL, in
which case only the number of items that would be stored in the array is returned.
Be nice and do the same here.
C.f. 0543105b0f.
This makes if /run/systemd/{seats,sessions,users} are missing, then
sd_get_seats(), sd_get_sessions() and sd_get_uids() return 0, that is,
an empty list, instead of -ENOENT.
The -ENOMEDIUM return value was introduced in v232-1001-g2977724b09,
('core: make hybrid cgroup unified mode keep compat /sys/fs/cgroup/systemd hierarchy'),
and would be returned by cg_pid_get_path_shifted(), but the documented and
expected return value is -ENODATA. Let's just catch ENXIO/ENOMEDIUM and translate
it to ENODATA in all cases.
Complements 171f8f591f, fixes#6012.
test-login.c is largely rewritten to use _cleanup_ and give more meaningful
messages (function names are used instead of creative terms like "active
session", so that when something unexpected is returned, it's much easier to
see what function is responsible).
The monitoring part is only activated if '-m' is passed on the command line.
It runs against the information from /run/systemd/ in the live system, but that
should be OK: logind/sd-login interface is supposed to be stable and both
backwards and forwards compatible.
If not running in a login session, some tests are skipped.
Those two changes together mean that it's possible to run test-login in the
test suite.
Tests for sd_pid_get_{unit,user_unit,slice} are added.
