Let's define two helpers strdupa_safe() + strndupa_safe() which do the
same as their non-safe counterparts, except that they abort if called
with allocations larger than ALLOCA_MAX.
This should ensure that all our alloca() based allocations are subject
to this limit.
afaics glibc offers three alloca() based APIs: alloca() itself,
strndupa() + strdupa(). With this we have now replacements for all of
them, that take the limit into account.
Also,
- drop unnecessary +1 from buffer size, as IF_NAMESIZE or IFNAMSIZ
includes the nul at the end.
- format_ifname() does not update buffer on failure,
- introduces format_ifname_alloc(), FORMAT_IFNAME(), and their friends.
Previously we only allows http/https urls, let's open this up a bit.
Why? Because it makes testing *so* *much* *easier* as we don't need to
run a HTTP server all the time.
CURL mostly abstracts the differences of http/https away from us, hence
we can get away with very little extra work.
In general we almost never hit those asserts in production code, so users see
them very rarely, if ever. But either way, we just need something that users
can pass to the developers.
We have quite a few of those asserts, and some have fairly nice messages, but
many are like "WTF?" or "???" or "unexpected something". The error that is
printed includes the file location, and function name. In almost all functions
there's at most one assert, so the function name alone is enough to identify
the failure for a developer. So we don't get much extra from the message, and
we might just as well drop them.
Dropping them makes our code a tiny bit smaller, and most importantly, improves
development experience by making it easy to insert such an assert in the code
without thinking how to phrase the argument.
We recently started making more use of malloc_usable_size() and rely on
it (see the string_erase() story). Given that we don't really support
sytems where malloc_usable_size() cannot be trusted beyond statistics
anyway, let's go fully in and rework GREEDY_REALLOC() on top of it:
instead of passing around and maintaining the currenly allocated size
everywhere, let's just derive it automatically from
malloc_usable_size().
I am mostly after this for the simplicity this brings. It also brings
minor efficiency improvements I guess, but things become so much nicer
to look at if we can avoid these allocation size variables everywhere.
Note that the malloc_usable_size() man page says relying on it wasn't
"good programming practice", but I think it does this for reasons that
don't apply here: the greedy realloc logic specifically doesn't rely on
the returned extra size, beyond the fact that it is equal or larger than
what was requested.
(This commit was supposed to be a quick patch btw, but apparently we use
the greedy realloc stuff quite a bit across the codebase, so this ends
up touching *a*lot* of code.)
With some versions of the compiler, the _cleanup_ attr makes it think
the variable might be freed/closed when uninitialized, even though it
cannot happen. The added cost is small enough to be worth the benefit,
and optimized builds will help reduce it even further.
As suggested in https://github.com/systemd/systemd/pull/11484#issuecomment-775288617.
This does not touch anything exposed in src/systemd. Changing the defines there
would be a compatibility break.
Note that tests are broken after this commit. They will be fixed in the next one.
The old name originates when this was used to discover "machine" images,
as managed by machined/machinectl. But nowadays this is also used by
portable services and system extensions, hence let's use a more generic
name for this API. Taking inspiration from "dissect-image.[ch]", let's call
this "discover-image.[ch]".
This is pure renaming, no other changes.
I think this formatting was originally used because it simplified
adding new options to the help messages. However, these days, most
tools their help message end with "\nSee the %s for details.\n" so
the final line almost never has to be edited which eliminates the
benefit of the custom formatting used for printf() help messages.
Let's make things more consistent and use the same formatting for
printf() help messages that we use everywhere else.
Prompted by https://github.com/systemd/systemd/pull/18355#discussion_r567241580
@keszybz's right on
https://github.com/systemd/systemd/pull/18248#issuecomment-760798473:
swapping out the userdata pointer of a live varlink connection is iffy.
Let's fix this by making the userdata inheritance from VarlinkServer
object to the Varlink connection object optional: we want it for most
cases, but not all, i.e. all those cases where the calls implemented as
varlink methods are stateless and can be answered synchronously. For the
other cases (i.e. where we want per-connection objects that wrap the
asynchronous operation as it goes on) let's not do such inheritance but
initialize the userdata pointer only once we have it. THis means the
original manager object must be manually retrieved from the
VarlinkServer object, which in turn needs to be requested from the
Varlink connection object.
The userdata inheritance is now controlled by the
VARLINK_INHERIT_USERDATA flag passed at VarlinkServer construction.
Alternative-to: #18248
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.
(Well, at least the ones where that makes sense. Where it does't make
sense are the ones that re invoked on the root path, which cannot
possibly be a symlink.)
Like it's customary in our codebase bus_error_message() internally takes
abs() of the passed error anyway, hence no need to explicitly negate it.
We mostly got this right, but in too many cases we didn't. Fix that.
Lennart Poettering
Yu Watanabe
Zbigniew Jędrzejewski-Szmek
David Tardon
Luca Boccassi
Daan De Meyer
Susant Sahani
Luca Boccassi
Frantisek Sumsal
fangxiuning
PhoenixDiscord