Skip to content
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

lib/deploy: Port final bootconfig writing to new style #1515

Closed
wants to merge 2 commits into from

Conversation

cgwalters
Copy link
Member

The main blocker for doing this before was the goto out handling
for remounting /boot. Handle that by factoring out the bits that
require it to a helper function, and do the C/GError equivalent of
"try/finally".

Not prep for anything right now, just decided to do this since I had the file
open.

* jump in and open a file there. See above TODO
* around doing this in a new mount namespace.
*/
int errsv = errno;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this isn't new, but that line confuses me. Was the intent here to save errno before the "finally" mount and restore it afterwards?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's more that in the past I've been bitten by the error handling functions themselves overwriting errno. Imagine if e.g. you want to do g_strdup_printf() to format an error message, and then go on to reference errno.. But in this case it's safe to drop.

The main blocker for doing this before was the `goto out` handling
for remounting `/boot`.  Handle that by factoring out the bits that
require it to a helper function, and do the C/GError equivalent of
"try/finally".

Not prep for anything right now, just decided to do this since I had the file
open.
@jlebon
Copy link
Member

jlebon commented Mar 26, 2018

@rh-atomic-bot r+ cf603f2

@rh-atomic-bot
Copy link

⌛ Testing commit cf603f2 with merge 2648c96...

@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: jlebon
Pushing 2648c96 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants