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

read-file.c: restore errno after successful fclose() #212

Conversation

eriksjolund
Copy link
Collaborator

@eriksjolund eriksjolund commented Oct 7, 2023

Revert any change to errno from a successful fclose(). According to errno(3) a function that succeeds is allowed to change errno.

So it should be enough to

diff --git a/tools/read-file.c b/tools/read-file.c
index ee35a93..9d98663 100644
--- a/tools/read-file.c
+++ b/tools/read-file.c
@@ -118,15 +118,15 @@ char *read_file(const char *path, size_t *length)
        buf = fread_file(f, length);
 
        save_errno = errno;
 
        if (fclose(f) != 0) {
                if (buf) {
                        save_errno = errno;
                        free(buf);
                }
                errno = save_errno;
                return NULL;
        }
-
+       errno = save_errno
        return buf;
 }

I rearranged that code somewhat.

Does this PR make sense? (I'm not 100% sure)

@eriksjolund eriksjolund force-pushed the restore-errno-after-successful-fclose branch from b6053d1 to 55c21dd Compare October 7, 2023 07:53
According to errno(3) a function that succeeds is allowed
to change errno.

Signed-off-by: Erik Sjölund <[email protected]>
@eriksjolund eriksjolund force-pushed the restore-errno-after-successful-fclose branch from 55c21dd to 0e0a644 Compare October 7, 2023 08:07
@giuseppe
Copy link
Member

giuseppe commented Oct 9, 2023

I am not really sure why we are saving the errno in this case.

Could you just drop the saved_errno logic in this function?

@alexlarsson
Copy link
Collaborator

We need to save errno to ensure that if there was a read-error reading the file in fread_file() we still report that when returning a NULL buf. Otherwise fclose() may override it.

@alexlarsson alexlarsson merged commit ce6abac into containers:main Oct 9, 2023
8 checks passed
@eriksjolund eriksjolund deleted the restore-errno-after-successful-fclose branch March 2, 2024 12:14
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