Clean up ignore_signals() + default_signals() + sigaction_many() a bit:
make it unnecessary to explicitly terminate the signal list with -1.
Merge all three calls into a single function that is just called with
slightly different parameters. And eliminate an unnecessary extra
iteration in its inner for() loop.
No change in behaviour.
A good chunk of parse-util.[ch] has been about parsing parts per
hundred/thousand/ten-thousand. Let's split that out into its own file.
No code changes, just some shuffling around.
If the cleanup function returns the appropriate type, use that to reset the
variable. For other functions (usually the foreign ones which return void), add
an explicit value to reset to.
This causes a bit of code churn, but I think it might be worth it. In a
following patch static destructors will be called from a fuzzer, and this
change allows them to be called multiple times. But I think such a change might
help with detecting unitialized code reuse too. We hit various bugs like this,
and things are more obvious when a pointer has been set to NULL.
I was worried whether this change increases text size, but it doesn't seem to:
-Dbuildtype=debug:
before "tree-wide: return NULL from freeing functions":
-rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 4494520 Feb 16 15:06 build/systemd*
after "tree-wide: return NULL from freeing functions":
-rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 4494576 Feb 16 15:10 build/systemd*
now:
-rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 4494640 Feb 16 15:15 build/systemd*
-Dbuildtype=release:
before "tree-wide: return NULL from freeing functions":
-rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:09 build-rawhide/systemd*
after "tree-wide: return NULL from freeing functions":
-rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:10 build-rawhide/systemd*
now:
-rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:16 build-rawhide/systemd*
I would expect that the compiler would be able to elide the setting of a
variable if the variable is never used again. And this seems to be the case:
in optimized builds there is no change in size whatsoever. And the change in
size in unoptimized build is negligible.
Something strange is happening with size of libsystemd: it's bigger in
optimized builds. Something to figure out, but unrelated to this patch.
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.
Let's make sure we still look at the etags reported by http 304 (i.e.
the cache management code). Otherwise we won't properly realize we
already downloaded this before.
This fixes a bug introduced in 6792cbbcf8
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.
Follow-up for 133b34f69a where this was
forgotten.
While we are at it, bring the parameters into the same order as we
declare them in the PullRaw/PullTar objects, i.e. match them to the
canonical order.
After all we want to remove the top-level dir, and make it look like the
only subdir, hence propagate the attributes from the subdir to the
top-level dir.
This merges the two flags that are passed to the ImportTar/ImportRaw
objects into a single flags parameter, which we then can extend more
easily later on.
No change in behaviour.
This is inspired by 133b34f69a which does
the same for PullTar/PullRaw.
We already had support for downlading a .nspawn and a .roothash file,
let's make the set complete, and also download .verity + roothash.p7s if
it exists, as nspawn consumes that.
Since there are now four kinds of additional resources to acquire, let's
introduce a PullFlags flags value for this instead of separate 'bool'
variables, it's just too many to always pass those around on the
function parameter list.
Previously we'd collect the data from redirects too, which wasn't
particularly terrible, since these typically don't carry the data we
were interested in, but it's still incorrect to do so.
Properly reset all fields that have to do with the current GET job when
we restart things. Previously we freed/reset only some stuff, leaking
some memory even.
Previously the PullJob object took internal care of rerequested the
SHA256SUMS file, if requesting <image>.sha256 didn't work. This was a
weird a non-abstraction only used when actually getting the checksum
files.
Let's move this out of the PullJob, so that it is generic again, and
does roughly the same stuff for all resources it is used for: let's
define a generic .on_not_found() handler that can be set on a PullJob
object, and is called whenever with see HTTP 404, and may be used to
provide a new URL to try if the first didn't work.
This is also preparation for later work to support PKCS#7 signatures
instead of gpg signatures, where a similar logic is needed, and we thus
should have a generic infrastructure place.
This gets rid of the VerificationStyle field in the PullJob object:
instead of storing this non-generic field we just derive the same
information from the URL itself, which is safe, since we generated it
ourselves earlier.
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.
Similar to free_and_replace. I think this should be uppercase to make it
clear that this is a macro. free_and_replace should probably be uppercased
too.
By default GNU tar will only read the first archive if multiple archives
are concatenated and ignore the rest. If an archive contains trailing
garbage this will hence not be recognized by tar as error, it simply
stops reading when the first archive is done (which might escalate to
SIGPIPE when invoked via a pipe).
Let's add --ignore-zeros to the tar command line when extracting. This
means:
1) if a tar archive was concatenated (i.e. generated with tar -A) we'll
process it correctly.
2) if a tar archive contains trailing garbage tar will now generate an
error message about it, instead of just throwing EPIPE, which makes
things easier to debug as broken files are not silently processed.
I think it's OK for gnu tar to ignore trailing garbage when dealing with
classic tapes drives, i.e. mediums that do not have a size limit
built-in. However, this is not what we are dealing with: we are dealing
with OS images here, that hopefully someone generated with a clean build
system, that were signed and validated and hence should not contain
trailing garbage. Hence it's better to refuse and complain thant to
silently eat up like for classic tape drives.
Fixes: #16605