sysroot: Reword comment and use gboolean over bool, error handling #3195
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ericcurtin
force-pushed
the
pr3192_comments
branch
from
1c0943c
to
b0f37b7
Compare
github-actions
bot
added
the
area/prepare-root
ericcurtin
force-pushed
the
pr3192_comments
branch
3 times, most recently
from
24a907e
to
2aebb28
Compare
ericcurtin
force-pushed
the
pr3192_comments
branch
4 times, most recently
from
b755a5c
to
df3bb0e
Compare
jlebon
reviewed
Feb 22, 2024
| * the `ostree=` karg for `ostree-prepare-root.service` specifically, but | ||
| * otherwise that karg doesn't exist on the real command-line. */ | ||
| if (!otcore_get_ostree_target (cmdline, &is_aboot, &ostree_target, | ||
| &otcore_get_ostree_target_error)) | ||
| return TRUE; | ||
| errx (EXIT_FAILURE, "Failed to determine ostree target: %s", |
There was a problem hiding this comment.
| error)) | ||
| if (is_aboot) | ||
| { | ||
| if (!_ostree_sysroot_parse_bootlink_aboot (ostree_target, &stateroot, error)) |
There was a problem hiding this comment.
Yeah, I think having it be separate is cleaner indeed. 👍
ericcurtin
force-pushed
the
pr3192_comments
branch
5 times, most recently
from
e66af4e
to
8b20427
Compare
ericcurtin
changed the title
jlebon
approved these changes
Feb 23, 2024
| /* This could happen in CoreOS live environments, where we hackily mock | ||
| gboolean is_aboot = false; | ||
| if (!otcore_get_ostree_target (cmdline, &is_aboot, &ostree_target, error)) | ||
| errx (EXIT_FAILURE, "Failed to determine ostree target: %s", (*error)->message); |
There was a problem hiding this comment.
Suggested change
| errx (EXIT_FAILURE, "Failed to determine ostree target: %s", (*error)->message); | |
| return glnx_prefix_error (error, "Invalid ostree target"); |
| static GRegex *regex; | ||
| g_autofree char *symlink_val = glnx_readlinkat_malloc (-1, bootlink, NULL, error); | ||
| if (!symlink_val) | ||
| return glnx_throw (error, "Failed to read '%s' symlink", bootlink); |
There was a problem hiding this comment.
Suggested change
| return glnx_throw (error, "Failed to read '%s' symlink", bootlink); | |
| return glnx_prefix_error (error, "Failed to read '%s' symlink", bootlink); |
Be more explicit in the comment, and use gboolean over bool. Less header inclusions when we use gboolean. Although bool is used in some places. Write a separate _ostree_sysroot_parse_bootlink_aboot function for aboot. Make is_aboot optional. Handle invalid androidboot karg and no ostree and androidboot kargs differently. Co-authored-by: Jonathan Lebon <jonathan@jlebon.com> Signed-off-by: Eric Curtin <ecurtin@redhat.com>
ericcurtin
force-pushed
the
pr3192_comments
branch
from
8b20427
to
e48cdb9
Compare
|
Re-pushed with changes |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Be more explicit in the comment, and use gboolean over bool. Less header
inclusions when we use gboolean. Although bool is used in some places.
Write a separate _ostree_sysroot_parse_bootlink_aboot function for
aboot. Make is_aboot optional. Handle invalid androidboot karg and no
ostree and androidboot kargs differently.
Co-authored-by: Jonathan Lebon jonathan@jlebon.com
Signed-off-by: Eric Curtin ecurtin@redhat.com