This allows callers to be checked for mismatches between format string
and arguments, and also means gcc can assume that the format string and
the arguments match up correctly when forwarding them to functions
like g_strdup_vprintf, removing the need to suppress -Wformat-nonliteral
warnings.
Signed-off-by: Simon McVittie <smcv@collabora.com>
In principle this could have been subject to a format string attack
via an argument containing %n, although in practice the code that uses
this format string is #if 0.
Signed-off-by: Simon McVittie <smcv@collabora.com>
If we use x86_64 and i386 in places where any architecture would do,
it's harder to recognise whether the test is assuming every machine
is x86 and would fail elsewhere (e.g. as fixed in #4142). When just
parsing abstract strings where any syntactically valid architecture
would do, let's use rare/niche architectures that none of the maintainers
are running on, so that hard-coding x86_64 or i386 stands out more.
I've used mips64 and m68k because they happen to be the same length as
x86_64 and i386, avoiding re-indentation.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Currenly we only list arch in the list of things to install from a
transaction if not all the refs are the same arch. It makes more sense
to show the arch even if its unique if they are not the primary arch.
It turns out that we can't currently uninstall a ref from a
non-standard arch without specifying the arch even if there is no similar
ref installed for the main arch. (#4264)
The fundamental reason for this that `flatpak_dir_find_installed_ref(s)`
currently only returns refs with standard arches unless you explicitly
specify an arch.
This changes flatpak_dir_find_installed_refs() to always return
all the refs for all installed arches. This is generally what
we want anyway, except in the case of "flatpak run org.some.Platform" where
we don't want to prompt if there are multiple arches installed, so that
is manually changed.
This changes find_matching_ref() to look for refs in all arches, but
always prefer (without prompting) the default arch if that is installed.
This also matches what all current callers want.
Fixes#4264
By default we only download the main arch subsummary, so if you added
a ref for some other arch it failed to find the ref. This works with the
CLI, because it explicilty loads the subsummary when its trying to expand
the parial ref to the full ref. However apps using libflatpak don't do that
so they failed.
If the tests are run as root, the system helper is not used even when
commands are run with `--system`. This means a different cache path is
used (see `_flatpak_dir_ensure_repo()`). The tests did not accommodate
for that.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
If looking up the summary for a ref without an arch (for example,
`ostree-metadata`, which the Endless OS version of flatpak uses in some
backwards-compatibility code), avoid passing `NULL` to `strcmp()` and
hence crashing.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Without this change, validate_component() might free the "component"
variable, but then go on to keep using it. This change also makes the
code work as intended, to only have a flatpak-specific "bundle" in the
appstream xml (where here bundle does not mean "flatpak single-file
bundle").
This is an optimized version of ostree_repo_prune() specialized for
archive mode repos. It is faster and uses less memory so that we can
prune larger repos (like flathub) in a realistic timeframe.
The primary reason it is faster is that it creates and uses a
`.commitmeta2` file for each commit, containing information about what
objects are reachable from that commit. This means incremental prunes
need only traverse over newly created commits.
Secondly, it uses the variant parser compiled accessors for the
various GVariants that are involved in the prune which is quite a bit
faster, especially if the repo is very large.
It also merges the scan-for-all-objects and prune-unreachable objects
phases, which means that we don't have to allocate a hashtable for
all the objects in the entire repo saving a lot of memory.
To save memory the hashtable of reachable objects, which can be quite
big on a big repo, points to a custom, very compact format for object
names.
Additionally it does the scanning for reachable objects twice, first
with a shared lock and then again (if anything changed) it with an
exclusive lock. This allows us to avoid using an exclusive lock during
the slowest part of the prune.
Unfortunately there are currently no public APIs for the ostree repo
locks. We really need to take an exclusive lock during the whole prune
or we parallel modifications (say a commit) might get their newly
written objects deleted. To work around this we have a minimal custom
implementation of an exclusive lock. Once the public API is available
we can start using that.
I created a repo with a lot of small commits to test this. It has 9M,
and pruning with depth=10 deletes 2M of them.
The original performance looks like:
Finding reachable objects: 287 seconds
Pruning unreachable: 69 seconds
Just using the pregenerated reachable data:
Finding reachable objects: 15 seconds
Pruning unreachable: 69 seconds
The final optimized prune (using pregenerated data):
Finding reachable objects: 12 seconds
Pruning unreachable: 51 seconds
The above are with the page caches cleaned, on a second run the performance
increase is even more noticeable.
As a comparison to the above, finding the reachable objects in the
actual flathub repo took 22 hours, but with the pregenerated reachable data
only 39 minutes.
scan-build points out that bytes isn't read after it is assigned. While
this is not actually true (scan-build doesn't understand
__attribute__((__cleanup__)), which frees bytes), it's true that we
should ideally have an assertion here.
Signed-off-by: Simon McVittie <smcv@collabora.com>
scan-build complained that rest_argv_start could be used uninitialized,
because it can't see that rest_argc >= 2 implies that rest_argv_start
got initialized at the same time rest_argc was set. Make this easier
to understand.
Signed-off-by: Simon McVittie <smcv@collabora.com>
scan-build detected that res was written but never read. Presumably
the use of ref here (carried over from the previous test) is a
copy/paste error.
Signed-off-by: Simon McVittie <smcv@collabora.com>
We always set match_len before using it, discarding the result of this
assignment. Detected by scan-build.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This hasn't done anything useful since 0978826c: it just takes a
new ref to the installation, and then releases that ref without doing
anything with it. Detected by scan-build.
Signed-off-by: Simon McVittie <smcv@collabora.com>
scan-build has a lot of false positives for this codebase because it
doesn't understand __attribute__((__cleanup__)) or GLib's GError
convention, but it seems to have been right about these.
Signed-off-by: Simon McVittie <smcv@collabora.com>
scan-build detected that mark_op_resolved() can be called with
op->resolved_commit == commit, in which case we incorrectly freed the
string before allocating the new copy.
Signed-off-by: Simon McVittie <smcv@collabora.com>
scan-build detected that response_size is uninitialized here, presumably
a typo for response_data_size.
Signed-off-by: Simon McVittie <smcv@collabora.com>
The intention here seems to have been that failing to close the http
stream provokes a warning but does not make the function fail, but we
were setting the wrong error, resulting in a NULL dereference if closing
the http stream somehow fails.
Signed-off-by: Simon McVittie <smcv@collabora.com>
In D-Bus, handles are defined to be unsigned, but in GVariant, for some
reason they're signed. Make sure they aren't negative, which could
result in a NULL dereference for fds.
A handle used in the conventional way will never legitimately be
negative (in GVariant's interpretation) or have its high bit set
(in D-Bus' interpretation), because file descriptors are signed 32-bit
integers, so an array of distinct file descriptors can never be long
enough for the distinction between signed and unsigned to matter.
In practice fds are limited by the kernel to several orders of
magnitude fewer than that anyway.
Fixes: 3ebf371f "run: Allow caller to replace /app and/or /usr"
Signed-off-by: Simon McVittie <smcv@collabora.com>
Like $XDG_RUNTIME_DIR/app/$FLATPAK_ID, this is shared between all
instances of the app, except for subsandboxed instances created by
flatpak-spawn --sandbox or equivalent. Unlike
$XDG_RUNTIME_DIR/app/$FLATPAK_ID, it does not exist at an equivalent
path on the host and in the sandboxed app.
Resolves: https://github.com/flatpak/flatpak/issues/4120
Signed-off-by: Simon McVittie <smcv@collabora.com>
If XDG_RUNTIME_DIR is under app control, as it will be with #4120, we
don't want to be mounting pieces of filesystem directly into it, because
that will mean that the app could create a symlink that will cause us
to create a mount point for it at the target of the symlink, potentially
elsewhere in the host filesystem.
Instead, we mount them in /run/flatpak, which is a per-instance
directory entirely controlled by Flatpak; and then create (relative)
symlinks in XDG_RUNTIME_DIR, pointing into /run/flatpak.
In this commit, we still know that the XDG_RUNTIME_DIR is a
per-instance tmpfs, so we can safely create the symlinks using
the --symlink option. In a subsequent commit this will change to
creating them in a shared XDG_RUNTIME_DIR, if any.
Signed-off-by: Simon McVittie <smcv@collabora.com>
