* [LTP] [PATCH v1] lib/safe_file_ops.c: Fix resource leak
@ 2022-02-11 14:20 Bogdan Lezhepekov via ltp
2022-02-15 12:59 ` Cyril Hrubis
0 siblings, 1 reply; 3+ messages in thread
From: Bogdan Lezhepekov via ltp @ 2022-02-11 14:20 UTC (permalink / raw)
To: ltp@lists.linux.it
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;
}
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,
--
2.35.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [LTP] [PATCH v1] lib/safe_file_ops.c: Fix resource leak
2022-02-11 14:20 [LTP] [PATCH v1] lib/safe_file_ops.c: Fix resource leak Bogdan Lezhepekov via ltp
@ 2022-02-15 12:59 ` Cyril Hrubis
0 siblings, 0 replies; 3+ messages in thread
From: Cyril Hrubis @ 2022-02-15 12:59 UTC (permalink / raw)
To: Bogdan Lezhepekov; +Cc: ltp@lists.linux.it
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
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 3+ messages in thread
* [LTP] [PATCH v1] lib/safe_file_ops.c: Fix resource leak
@ 2022-02-11 16:24 Bogdan Lezhepekov
0 siblings, 0 replies; 3+ messages in thread
From: Bogdan Lezhepekov @ 2022-02-11 16:24 UTC (permalink / raw)
To: ltp
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 | 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;
}
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,
--
2.35.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-02-15 12:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-11 14:20 [LTP] [PATCH v1] lib/safe_file_ops.c: Fix resource leak Bogdan Lezhepekov via ltp
2022-02-15 12:59 ` Cyril Hrubis
-- strict thread matches above, loose matches on Subject: below --
2022-02-11 16:24 Bogdan Lezhepekov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox