| Message ID | 20230830060858.2841-1-dingxiang@cmss.chinamobile.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/landlock: Fix a resource leak | expand |
Hi, Thanks for this fix. A few suggestions: On Wed, Aug 30, 2023 at 02:08:58PM +0800, Ding Xiang wrote: > The opened file should be closed before return, > otherwise resource leak will occur > > Signed-off-by: Ding Xiang <dingxiang@cmss.chinamobile.com> > --- > tools/testing/selftests/landlock/fs_test.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c > index 83d565569512..687a66ea9799 100644 > --- a/tools/testing/selftests/landlock/fs_test.c > +++ b/tools/testing/selftests/landlock/fs_test.c > @@ -124,13 +124,17 @@ static bool supports_filesystem(const char *const filesystem) > return true; > > /* filesystem can be null for bind mounts. */ > - if (!filesystem) > + if (!filesystem) { > + fclose(inf); > return true; Can you please change these two lines (and the next hunk) with a `goto out` (and then don't add the if braces), initialize res to true when it is declared, and add the `out` label below? > + } > > len = snprintf(str, sizeof(str), "nodev\t%s\n", filesystem); > - if (len >= sizeof(str)) > + if (len >= sizeof(str)) { > + fclose(inf); > /* Ignores too-long filesystem names. */ > return true; > + } > > res = fgrep(inf, str); out: > fclose(inf); > -- > 2.38.1 > > >
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 83d565569512..687a66ea9799 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -124,13 +124,17 @@ static bool supports_filesystem(const char *const filesystem) return true; /* filesystem can be null for bind mounts. */ - if (!filesystem) + if (!filesystem) { + fclose(inf); return true; + } len = snprintf(str, sizeof(str), "nodev\t%s\n", filesystem); - if (len >= sizeof(str)) + if (len >= sizeof(str)) { + fclose(inf); /* Ignores too-long filesystem names. */ return true; + } res = fgrep(inf, str); fclose(inf);
The opened file should be closed before return, otherwise resource leak will occur Signed-off-by: Ding Xiang <dingxiang@cmss.chinamobile.com> --- tools/testing/selftests/landlock/fs_test.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)