-
Notifications
You must be signed in to change notification settings - Fork 305
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
repo/checkout: Finish conversion to new code style #792
Conversation
I plan to make some future changes here, and wanted to do this first. Random side note; how about converting the do/while loops for `EINTR` to `TEMP_FAILURE_RETRY()`? We're very inconsistent about that...
src/libostree/ostree-repo-checkout.c
Outdated
checksum = ostree_repo_file_get_checksum ((OstreeRepoFile*)source); | ||
|
||
is_whiteout = !is_symlink && options->process_whiteouts && | ||
const gboolean is_symlink = g_file_info_get_file_type (source_info) == G_FILE_TYPE_SYMBOLIC_LINK; |
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.
Can we wrap in parentheses?
src/libostree/ostree-repo-checkout.c
Outdated
is_whiteout = !is_symlink && options->process_whiteouts && | ||
const gboolean is_symlink = g_file_info_get_file_type (source_info) == G_FILE_TYPE_SYMBOLIC_LINK; | ||
const char *checksum = ostree_repo_file_get_checksum ((OstreeRepoFile*)source); | ||
const gboolean is_whiteout = !is_symlink && options->process_whiteouts && |
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.
And here also!
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.
Note this was just a moved hunk 🧀, not net new code. But sure. Fixups ⬇️
|
||
if (hardlink_res == HARDLINK_RESULT_LINKED && options->devino_to_csum_cache) | ||
{ | ||
struct stat stbuf; | ||
OstreeDevIno *key; | ||
|
||
|
||
if (TEMP_FAILURE_RETRY (fstatat (destination_dfd, destination_name, &stbuf, AT_SYMLINK_NOFOLLOW)) != 0) |
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.
Re. your comment, I think my vote is for TEMP_FAILURE_RETRY
. I find it easier to read (plus, it's even less boilerplate!).
src/libostree/ostree-repo-checkout.c
Outdated
g_file_info_get_name (source_info), | ||
cancellable, error); | ||
goto out; | ||
if (!checkout_one_file_at (self, options, |
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.
We can just return
the value directly, right?
☀️ Test successful - status-atomicjenkins |
I plan to make some future changes here, and wanted to do this
first.
Random side note; how about converting the do/while loops for
EINTR
toTEMP_FAILURE_RETRY()
? We're very inconsistent about that...