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

add checking of return status on fstat calls #7

Merged
merged 1 commit into from
May 21, 2024

Conversation

ncroxon
Copy link
Contributor

@ncroxon ncroxon commented May 15, 2024

There are a few places we don't check the return status when calling fstat for success. Clean up the calls by adding a check before continuing.

V2: clean up warnings/errors from checkpatch.pl

Copy link
Member

@mtkaczyk mtkaczyk left a comment

Choose a reason for hiding this comment

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

Hey @ncroxon,
There is one small issue. Please fix it and we are ready to go :)

Assemble.c Outdated

if (fstat(mdfd, &stb2) != 0) {
goto error;
}
Copy link
Member

Choose a reason for hiding this comment

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

You don't need braces in this case, there is one instruction inside if but it is not an issue, just FYI,

Dump.c Outdated
@@ -110,9 +111,17 @@ int Dump_metadata(char *dev, char *dir, struct context *c,
free(fname);
return 1;
}
if (c->verbose >= 0)
if (c->verbose >= 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Oh, bracases here totally change the flow.

Previously, only printf() was executed verbose >=0. Now, the fsttat is executed. I think that it should be:

Suggested change
if (c->verbose >= 0) {
if (c->verbose >= 0)
printf("%s saved as %s.\n", dev, fname);
close(fl);
ret = fstat(fd, &dstb);
close(fd);
if (ret) {
unlink(fname);
free(fname);
return 1;
}

@ncroxon
Copy link
Contributor Author

ncroxon commented May 16, 2024

I pushed a new version (V3) to the branch.

@mtkaczyk
Copy link
Member

[PATCH V3] add checking of return status on fstat calls
There are a few places we don't check the return status when
calling fstat for success. Clean up the calls by adding a
check before continuing.

V3: fix Dump.c braces to execute verbose properly.
    clean up braces with only one instruction inside.
V2: clean up warnings/errors from checkpatch.pl

Signed-off-by: Nigel Croxon <ncroxon@redhat.com>

I will cherry-pick this commit so please remove [PATCH V3] from title and v2 and v3 changes from description.
We have to use comment to note changes from now.. thanks!

There are a few places we don't check the return status when
calling fstat for success. Clean up the calls by adding a
check before continuing.

Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
@mtkaczyk mtkaczyk merged commit 95673c7 into md-raid-utilities:main May 21, 2024
8 checks passed
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.

None yet

2 participants