| Message ID | 20250222121249.50588-1-ritvikfoss@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] selftests/mount: Close 'fd' when write fails | expand |
On 25/02/22 05:42PM, ritvikfoss@gmail.com wrote: > From: Ritvik Gupta <ritvikfoss@gmail.com> > > 1. Close the file descriptor when write fails. > 2. Introduce 'close_or_die' helper function to > reduce repetition. > > Signed-off-by: Ritvik Gupta <ritvikfoss@gmail.com> > --- > Changes in v2: > - Fixed formatting > > .../selftests/mount/unprivileged-remount-test.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/mount/unprivileged-remount-test.c b/tools/testing/selftests/mount/unprivileged-remount-test.c > index d2917054fe3a..41d7547c781d 100644 > --- a/tools/testing/selftests/mount/unprivileged-remount-test.c > +++ b/tools/testing/selftests/mount/unprivileged-remount-test.c > @@ -54,6 +54,14 @@ static void die(char *fmt, ...) > exit(EXIT_FAILURE); > } > > +static void close_or_die(char *filename, int fd) > +{ > + if (close(fd) != 0) { > + die("close of %s failed: %s\n", > + filename, strerror(errno)); > + } > +} > + > static void vmaybe_write_file(bool enoent_ok, char *filename, char *fmt, va_list ap) > { > char buf[4096]; > @@ -79,6 +87,7 @@ static void vmaybe_write_file(bool enoent_ok, char *filename, char *fmt, va_list > } > written = write(fd, buf, buf_len); > if (written != buf_len) { > + close_or_die(filename, fd); > if (written >= 0) { > die("short write to %s\n", filename); > } else { > @@ -86,10 +95,7 @@ static void vmaybe_write_file(bool enoent_ok, char *filename, char *fmt, va_list > filename, strerror(errno)); > } > } > - if (close(fd) != 0) { > - die("close of %s failed: %s\n", > - filename, strerror(errno)); > - } > + close_or_die(filename, fd); > } > > static void maybe_write_file(char *filename, char *fmt, ...) > -- > 2.48.1 > > Closing a file right before a process exits is redundant, since the kernel will clean it up automatically anyway. That said, whether doing this as a best practice is arguable. Cheers, Seyediman
diff --git a/tools/testing/selftests/mount/unprivileged-remount-test.c b/tools/testing/selftests/mount/unprivileged-remount-test.c index d2917054fe3a..41d7547c781d 100644 --- a/tools/testing/selftests/mount/unprivileged-remount-test.c +++ b/tools/testing/selftests/mount/unprivileged-remount-test.c @@ -54,6 +54,14 @@ static void die(char *fmt, ...) exit(EXIT_FAILURE); } +static void close_or_die(char *filename, int fd) +{ + if (close(fd) != 0) { + die("close of %s failed: %s\n", + filename, strerror(errno)); + } +} + static void vmaybe_write_file(bool enoent_ok, char *filename, char *fmt, va_list ap) { char buf[4096]; @@ -79,6 +87,7 @@ static void vmaybe_write_file(bool enoent_ok, char *filename, char *fmt, va_list } written = write(fd, buf, buf_len); if (written != buf_len) { + close_or_die(filename, fd); if (written >= 0) { die("short write to %s\n", filename); } else { @@ -86,10 +95,7 @@ static void vmaybe_write_file(bool enoent_ok, char *filename, char *fmt, va_list filename, strerror(errno)); } } - if (close(fd) != 0) { - die("close of %s failed: %s\n", - filename, strerror(errno)); - } + close_or_die(filename, fd); } static void maybe_write_file(char *filename, char *fmt, ...)