-
Notifications
You must be signed in to change notification settings - Fork 297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sysroot: Reword comment and use gboolean over bool, error handling #3195
Conversation
1c0943c
to
b0f37b7
Compare
24a907e
to
2aebb28
Compare
b755a5c
to
df3bb0e
Compare
* 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
error)) | ||
if (is_aboot) | ||
{ | ||
if (!_ostree_sysroot_parse_bootlink_aboot (ostree_target, &stateroot, error)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think having it be separate is cleaner indeed. 👍
e66af4e
to
8b20427
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some last error-handling-related bits, otherwise LGTM.
/* 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 <[email protected]> Signed-off-by: Eric Curtin <[email protected]>
8b20427
to
e48cdb9
Compare
Re-pushed with changes |
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 [email protected]
Signed-off-by: Eric Curtin [email protected]