diff mbox series

[v2] lib/safe_file_ops.c: Fix resource leak

Message ID 20220216074800.22621-1-blezhepekov@suse.de
State Rejected
Headers show
Series [v2] lib/safe_file_ops.c: Fix resource leak | expand

Commit Message

Bogdan Lezhepekov Feb. 16, 2022, 7:48 a.m. UTC 
safe_file_scanf and safe_file_vprintf suffered
from resource leak, as opened file descriptor
was not closed in case of error.

Signed-off-by: Bogdan Lezhepekov <blezhepekov@suse.de>
---
 lib/safe_file_ops.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Petr Vorel Feb. 16, 2022, 8:51 a.m. UTC | #1 
Hi Bogdan,

> safe_file_scanf and safe_file_vprintf suffered
> from resource leak, as opened file descriptor
> was not closed in case of error.
I guess we don't care about closing file descriptor in both safe_file_scanf()
and safe_file_vprintf() because test exits due used tst_brkm_().

And we *do* care in file_printf() with err: label because we don't exit there
(tst_resm_() does not exit).

But instead, you could replace fopen() / fclose() which is followed by
tst_brkm_() by safe_fopen() / safe_fclose() (to shorten the code).

Kind regards,
Petr
diff mbox series

Patch

diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c
index f803691d8..53711fb74 100644
--- a/lib/safe_file_ops.c
+++ b/lib/safe_file_ops.c
@@ -142,14 +142,14 @@  void safe_file_scanf(const char *file, const int lineno,
 	if (ret == EOF) {
 		tst_brkm_(file, lineno, TBROK, cleanup_fn,
 			"The FILE '%s' ended prematurely", path);
-		return;
+		goto out;
 	}
 
 	if (ret != exp_convs) {
 		tst_brkm_(file, lineno, TBROK, cleanup_fn,
 			"Expected %i conversions got %i FILE '%s'",
 			exp_convs, ret, path);
-		return;
+		goto out;
 	}
 
 	if (fclose(f)) {
@@ -157,6 +157,8 @@  void safe_file_scanf(const char *file, const int lineno,
 			"Failed to close FILE '%s'", path);
 		return;
 	}
+out:
+	fclose(f);
 }
 
 
@@ -267,7 +269,7 @@  static void safe_file_vprintf(const char *file, const int lineno,
 	if (vfprintf(f, fmt, va) < 0) {
 		tst_brkm_(file, lineno, TBROK, cleanup_fn,
 			"Failed to print to FILE '%s'", path);
-		return;
+		goto out;
 	}
 
 	if (fclose(f)) {
@@ -275,6 +277,8 @@  static void safe_file_vprintf(const char *file, const int lineno,
 			"Failed to close FILE '%s'", path);
 		return;
 	}
+out:
+	fclose(f);
 }
 
 void safe_file_printf(const char *file, const int lineno,