Messages in this thread | | | Date | Wed, 30 Aug 2023 11:01:47 +0200 | From | Mickaël Salaün <> | Subject | Re: [PATCH] selftests/landlock: Fix a resource leak |
| |
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 > > >
| |