| Message ID | VI1PR04MB49583062AFA205712ABA54B793309@VI1PR04MB4958.eurprd04.prod.outlook.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v1] lib/safe_file_ops.c: Fix resource leak | expand |
Hi! > 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 <bogdan.lezhepekov@suse.com> > --- > lib/safe_file_ops.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c > index f803691d8..d7231df4d 100644 > --- a/lib/safe_file_ops.c > +++ b/lib/safe_file_ops.c > @@ -130,7 +130,7 @@ void safe_file_scanf(const char *file, const int lineno, > if (f == NULL) { > tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn, > "Failed to open FILE '%s' for reading", path); > - return; > + goto out; This one is wrong, we just tested that f == NULL > } > > exp_convs = tst_count_scanf_conversions(fmt); > @@ -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; Technically this tst_brkm_() call does not return unless we are in the test cleanup. During the rest of the test execution it ends up calling exit(TBROK). So this return is reached only if something fails during the test cleanup. I guess that we can fix it like this, but the test will exit shortly after anyways. > + 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); > } > > > @@ -261,13 +263,13 @@ static void safe_file_vprintf(const char *file, const int lineno, > if (f == NULL) { > tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn, > "Failed to open FILE '%s' for writing", path); > - return; > + goto out; And here as well. > } > > 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, > -- > 2.35.1 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c index f803691d8..d7231df4d 100644 --- a/lib/safe_file_ops.c +++ b/lib/safe_file_ops.c @@ -130,7 +130,7 @@ void safe_file_scanf(const char *file, const int lineno, if (f == NULL) { tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn, "Failed to open FILE '%s' for reading", path); - return; + goto out; } exp_convs = tst_count_scanf_conversions(fmt); @@ -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); } @@ -261,13 +263,13 @@ static void safe_file_vprintf(const char *file, const int lineno, if (f == NULL) { tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn, "Failed to open FILE '%s' for writing", path); - return; + goto out; } 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,
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 <bogdan.lezhepekov@suse.com> --- lib/safe_file_ops.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)