| Message ID | 20221220075714.28762-1-akumar@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5] statvfs01: Convert to new LTP API | expand |
Hello, Avinesh Kumar <akumar@suse.de> writes: > Removed the TINFO statements, > Added a validation of statvfs.f_namemax field by trying to create > files of valid and invalid length names. > > Changes in v5: > Enabled file creation for vfat and exfat filesystems also. > > Signed-off-by: Avinesh Kumar <akumar@suse.de> Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com> > --- > testcases/kernel/syscalls/statvfs/statvfs01.c | 109 +++++++----------- > 1 file changed, 40 insertions(+), 69 deletions(-) > > diff --git a/testcases/kernel/syscalls/statvfs/statvfs01.c b/testcases/kernel/syscalls/statvfs/statvfs01.c > index e3b356c93..034835da7 100644 > --- a/testcases/kernel/syscalls/statvfs/statvfs01.c > +++ b/testcases/kernel/syscalls/statvfs/statvfs01.c > @@ -1,92 +1,63 @@ > +// SPDX-License-Identifier: GPL-2.0 > /* > * Copyright (c) Wipro Technologies Ltd, 2005. All Rights Reserved. > * AUTHOR: Prashant P Yendigeri <prashant.yendigeri@wipro.com> > - * > - * This program is free software; you can redistribute it and/or modify it > - * under the terms of version 2 of the GNU General Public License as > - * published by the Free Software Foundation. > - * > - * This program is distributed in the hope that it would be useful, but > - * WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > - * > - * You should have received a copy of the GNU General Public License along > - * with this program; if not, write the Free Software Foundation, Inc., > - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > - * > + * Copyright (c) 2022 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com> > */ > -/* > - * DESCRIPTION > - * This is a Phase I test for the statvfs(2) system call. > - * It is intended to provide a limited exposure of the system call. > - * This call behaves similar to statfs. > + > +/*\ > + * [Description] > + * > + * Verify that statvfs() executes successfully for all > + * available filesystems. Verify statvfs.f_namemax field > + * by trying to create files of valid and invalid length names. > */ > > -#include <stdio.h> > -#include <unistd.h> > -#include <errno.h> > #include <sys/statvfs.h> > -#include <stdint.h> > - > -#include "test.h" > +#include "tst_test.h" > > -#define TEST_PATH "/" > +#define MNT_POINT "mntpoint" > +#define TEST_PATH MNT_POINT"/testfile" > +#define NLS_MAX_CHARSET_SIZE 6 > > -static void setup(void); > -static void cleanup(void); > - > -char *TCID = "statvfs01"; > -int TST_TOTAL = 1; > - > -int main(int ac, char **av) > +static void run(void) > { > struct statvfs buf; > - int lc; > - > - tst_parse_opts(ac, av, NULL, NULL); > - > - setup(); > + char *valid_fname, *toolong_fname; > + long fs_type; > > - for (lc = 0; TEST_LOOPING(lc); lc++) { > + fs_type = tst_fs_type(TEST_PATH); > > - tst_count = 0; > + TST_EXP_PASS(statvfs(TEST_PATH, &buf)); > > - TEST(statvfs(TEST_PATH, &buf)); > + toolong_fname = SAFE_MALLOC(buf.f_namemax + 1); > + if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) > + valid_fname = SAFE_MALLOC(buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); > + else > + valid_fname = SAFE_MALLOC(buf.f_namemax - 1); > > - if (TEST_RETURN == -1) { > - tst_resm(TFAIL | TTERRNO, "statvfs(%s, ...) failed", > - TEST_PATH); > - } else { > - tst_resm(TPASS, "statvfs(%s, ...) passed", TEST_PATH); > - } > + memset(toolong_fname, 'b', buf.f_namemax + 1); > + if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) > + memset(valid_fname, 'a', buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); > + else > + memset(valid_fname, 'a', buf.f_namemax - 1); > > - } > + TST_EXP_FD(creat(valid_fname, 0444)); > + SAFE_CLOSE(TST_RET); > > - tst_resm(TINFO, "This call is similar to statfs"); > - tst_resm(TINFO, "Extracting info about the '%s' file system", > - TEST_PATH); > - tst_resm(TINFO, "file system block size = %lu bytes", buf.f_bsize); > - tst_resm(TINFO, "file system fragment size = %lu bytes", buf.f_frsize); > - tst_resm(TINFO, "file system free blocks = %ju", > - (uintmax_t) buf.f_bfree); > - tst_resm(TINFO, "file system total inodes = %ju", > - (uintmax_t) buf.f_files); > - tst_resm(TINFO, "file system free inodes = %ju", > - (uintmax_t) buf.f_ffree); > - tst_resm(TINFO, "file system id = %lu", buf.f_fsid); > - tst_resm(TINFO, "file system max filename length = %lu", buf.f_namemax); > - > - cleanup(); > - tst_exit(); > + TST_EXP_FAIL(creat(toolong_fname, 0444), ENAMETOOLONG); > } > > static void setup(void) > { > - tst_sig(NOFORK, DEF_HANDLER, cleanup); > - > - TEST_PAUSE; > + SAFE_TOUCH(TEST_PATH, 0666, NULL); > } > > -static void cleanup(void) > -{ > -} > +static struct tst_test test = { > + .test_all = run, > + .setup = setup, > + .needs_root = 1, > + .mount_device = 1, > + .mntpoint = MNT_POINT, > + .all_filesystems = 1 > +}; > -- > 2.39.0
Hi Avinesh, Richie, Generally LGTM, thanks for fixing exfat and vfat. > > + toolong_fname = SAFE_MALLOC(buf.f_namemax + 1); However, length could be smaller: Instead using buf.f_namemax + 1 (1531) also for exfat and vfat, invalid length is already buf.f_namemax / NLS_MAX_CHARSET_SIZE + 1 (256). > > + if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) > > + valid_fname = SAFE_MALLOC(buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); > > + else > > + valid_fname = SAFE_MALLOC(buf.f_namemax - 1); > > - if (TEST_RETURN == -1) { > > - tst_resm(TFAIL | TTERRNO, "statvfs(%s, ...) failed", > > - TEST_PATH); > > - } else { > > - tst_resm(TPASS, "statvfs(%s, ...) passed", TEST_PATH); > > - } > > + memset(toolong_fname, 'b', buf.f_namemax + 1); > > + if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) > > + memset(valid_fname, 'a', buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); > > + else > > + memset(valid_fname, 'a', buf.f_namemax - 1); Also valid length is for buf.f_namemax, not buf.f_namemax - 1. I guess -1 is for \0 (NULL terminator), but tests work even with just buf.f_namemax. Also adding variable to hold the length makes source more readable. How about this? struct statvfs buf; char *valid_fname, *toolong_fname; long fs_type; long valid_len; fs_type = tst_fs_type(TEST_PATH); TST_EXP_PASS(statvfs(TEST_PATH, &buf)); valid_len = buf.f_namemax; if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) valid_len = buf.f_namemax / NLS_MAX_CHARSET_SIZE; valid_fname = SAFE_MALLOC(valid_len); memset(valid_fname, 'a', valid_len); toolong_fname = SAFE_MALLOC(valid_len + 1); memset(toolong_fname, 'b', valid_len + 1); Final diff is below. Kind regards, Petr diff --git testcases/kernel/syscalls/statvfs/statvfs01.c testcases/kernel/syscalls/statvfs/statvfs01.c index 034835da7d..f357855eb1 100644 --- testcases/kernel/syscalls/statvfs/statvfs01.c +++ testcases/kernel/syscalls/statvfs/statvfs01.c @@ -25,22 +25,21 @@ static void run(void) struct statvfs buf; char *valid_fname, *toolong_fname; long fs_type; + long valid_len; fs_type = tst_fs_type(TEST_PATH); TST_EXP_PASS(statvfs(TEST_PATH, &buf)); - toolong_fname = SAFE_MALLOC(buf.f_namemax + 1); + valid_len = buf.f_namemax; if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) - valid_fname = SAFE_MALLOC(buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); - else - valid_fname = SAFE_MALLOC(buf.f_namemax - 1); + valid_len = buf.f_namemax / NLS_MAX_CHARSET_SIZE; - memset(toolong_fname, 'b', buf.f_namemax + 1); - if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) - memset(valid_fname, 'a', buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); - else - memset(valid_fname, 'a', buf.f_namemax - 1); + valid_fname = SAFE_MALLOC(valid_len); + memset(valid_fname, 'a', valid_len); + + toolong_fname = SAFE_MALLOC(valid_len + 1); + memset(toolong_fname, 'b', valid_len + 1); TST_EXP_FD(creat(valid_fname, 0444)); SAFE_CLOSE(TST_RET);
Hello, Petr Vorel <pvorel@suse.cz> writes: > Hi Avinesh, Richie, > > Generally LGTM, thanks for fixing exfat and vfat. > >> > + toolong_fname = SAFE_MALLOC(buf.f_namemax + 1); > However, length could be smaller: > Instead using buf.f_namemax + 1 (1531) also for exfat and vfat, > invalid length is already buf.f_namemax / NLS_MAX_CHARSET_SIZE + 1 > (256). > >> > + if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) >> > + valid_fname = SAFE_MALLOC(buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); >> > + else >> > + valid_fname = SAFE_MALLOC(buf.f_namemax - 1); There is also a memory leak when running with -I. You could just use an 4Kb (PATH_MAX) static buffer as the name. If we find an FS that allows longer names then we can increase it. We could also use a guarded buffer (specified in tst_test). > >> > - if (TEST_RETURN == -1) { >> > - tst_resm(TFAIL | TTERRNO, "statvfs(%s, ...) failed", >> > - TEST_PATH); >> > - } else { >> > - tst_resm(TPASS, "statvfs(%s, ...) passed", TEST_PATH); >> > - } >> > + memset(toolong_fname, 'b', buf.f_namemax + 1); >> > + if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) >> > + memset(valid_fname, 'a', buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); >> > + else >> > + memset(valid_fname, 'a', buf.f_namemax - 1); > Also valid length is for buf.f_namemax, not buf.f_namemax - 1. I guess -1 is for > \0 (NULL terminator), but tests work even with just buf.f_namemax. > > Also adding variable to hold the length makes source more readable. > How about this? > > struct statvfs buf; > char *valid_fname, *toolong_fname; > long fs_type; > long valid_len; > > fs_type = tst_fs_type(TEST_PATH); > > TST_EXP_PASS(statvfs(TEST_PATH, &buf)); > > valid_len = buf.f_namemax; > if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) > valid_len = buf.f_namemax / NLS_MAX_CHARSET_SIZE; > > valid_fname = SAFE_MALLOC(valid_len); > memset(valid_fname, 'a', valid_len); > > toolong_fname = SAFE_MALLOC(valid_len + 1); > memset(toolong_fname, 'b', valid_len + 1); > > Final diff is below. > > Kind regards, > Petr > > diff --git testcases/kernel/syscalls/statvfs/statvfs01.c testcases/kernel/syscalls/statvfs/statvfs01.c > index 034835da7d..f357855eb1 100644 > --- testcases/kernel/syscalls/statvfs/statvfs01.c > +++ testcases/kernel/syscalls/statvfs/statvfs01.c > @@ -25,22 +25,21 @@ static void run(void) > struct statvfs buf; > char *valid_fname, *toolong_fname; > long fs_type; > + long valid_len; > > fs_type = tst_fs_type(TEST_PATH); > > TST_EXP_PASS(statvfs(TEST_PATH, &buf)); > > - toolong_fname = SAFE_MALLOC(buf.f_namemax + 1); > + valid_len = buf.f_namemax; > if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) > - valid_fname = SAFE_MALLOC(buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); > - else > - valid_fname = SAFE_MALLOC(buf.f_namemax - 1); > + valid_len = buf.f_namemax / NLS_MAX_CHARSET_SIZE; > > - memset(toolong_fname, 'b', buf.f_namemax + 1); > - if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) > - memset(valid_fname, 'a', buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); > - else > - memset(valid_fname, 'a', buf.f_namemax - 1); > + valid_fname = SAFE_MALLOC(valid_len); > + memset(valid_fname, 'a', valid_len); > + > + toolong_fname = SAFE_MALLOC(valid_len + 1); > + memset(toolong_fname, 'b', valid_len + 1); > > TST_EXP_FD(creat(valid_fname, 0444)); > SAFE_CLOSE(TST_RET);
Hi Petr, Richie, Thank you for reviewing again and providing more improvement suggestions, I've made the changes in v6. Thanks, Avinesh On Thursday, December 22, 2022 3:23:35 PM IST Richard Palethorpe wrote: > Hello, > > Petr Vorel <pvorel@suse.cz> writes: > > > Hi Avinesh, Richie, > > > > Generally LGTM, thanks for fixing exfat and vfat. > > > >> > + toolong_fname = SAFE_MALLOC(buf.f_namemax + 1); > > However, length could be smaller: > > Instead using buf.f_namemax + 1 (1531) also for exfat and vfat, > > invalid length is already buf.f_namemax / NLS_MAX_CHARSET_SIZE + 1 > > (256). > > > >> > + if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) > >> > + valid_fname = SAFE_MALLOC(buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); > >> > + else > >> > + valid_fname = SAFE_MALLOC(buf.f_namemax - 1); > > There is also a memory leak when running with -I. You could just use an > 4Kb (PATH_MAX) static buffer as the name. If we find an FS that allows > longer names then we can increase it. > > We could also use a guarded buffer (specified in tst_test). > > > > >> > - if (TEST_RETURN == -1) { > >> > - tst_resm(TFAIL | TTERRNO, "statvfs(%s, ...) failed", > >> > - TEST_PATH); > >> > - } else { > >> > - tst_resm(TPASS, "statvfs(%s, ...) passed", TEST_PATH); > >> > - } > >> > + memset(toolong_fname, 'b', buf.f_namemax + 1); > >> > + if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) > >> > + memset(valid_fname, 'a', buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); > >> > + else > >> > + memset(valid_fname, 'a', buf.f_namemax - 1); > > Also valid length is for buf.f_namemax, not buf.f_namemax - 1. I guess -1 is for > > \0 (NULL terminator), but tests work even with just buf.f_namemax. > > > > Also adding variable to hold the length makes source more readable. > > How about this? > > > > struct statvfs buf; > > char *valid_fname, *toolong_fname; > > long fs_type; > > long valid_len; > > > > fs_type = tst_fs_type(TEST_PATH); > > > > TST_EXP_PASS(statvfs(TEST_PATH, &buf)); > > > > valid_len = buf.f_namemax; > > if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) > > valid_len = buf.f_namemax / NLS_MAX_CHARSET_SIZE; > > > > valid_fname = SAFE_MALLOC(valid_len); > > memset(valid_fname, 'a', valid_len); > > > > toolong_fname = SAFE_MALLOC(valid_len + 1); > > memset(toolong_fname, 'b', valid_len + 1); > > > > Final diff is below. > > > > Kind regards, > > Petr > > > > diff --git testcases/kernel/syscalls/statvfs/statvfs01.c testcases/kernel/syscalls/statvfs/statvfs01.c > > index 034835da7d..f357855eb1 100644 > > --- testcases/kernel/syscalls/statvfs/statvfs01.c > > +++ testcases/kernel/syscalls/statvfs/statvfs01.c > > @@ -25,22 +25,21 @@ static void run(void) > > struct statvfs buf; > > char *valid_fname, *toolong_fname; > > long fs_type; > > + long valid_len; > > > > fs_type = tst_fs_type(TEST_PATH); > > > > TST_EXP_PASS(statvfs(TEST_PATH, &buf)); > > > > - toolong_fname = SAFE_MALLOC(buf.f_namemax + 1); > > + valid_len = buf.f_namemax; > > if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) > > - valid_fname = SAFE_MALLOC(buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); > > - else > > - valid_fname = SAFE_MALLOC(buf.f_namemax - 1); > > + valid_len = buf.f_namemax / NLS_MAX_CHARSET_SIZE; > > > > - memset(toolong_fname, 'b', buf.f_namemax + 1); > > - if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) > > - memset(valid_fname, 'a', buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); > > - else > > - memset(valid_fname, 'a', buf.f_namemax - 1); > > + valid_fname = SAFE_MALLOC(valid_len); > > + memset(valid_fname, 'a', valid_len); > > + > > + toolong_fname = SAFE_MALLOC(valid_len + 1); > > + memset(toolong_fname, 'b', valid_len + 1); > > > > TST_EXP_FD(creat(valid_fname, 0444)); > > SAFE_CLOSE(TST_RET); > > >
diff --git a/testcases/kernel/syscalls/statvfs/statvfs01.c b/testcases/kernel/syscalls/statvfs/statvfs01.c index e3b356c93..034835da7 100644 --- a/testcases/kernel/syscalls/statvfs/statvfs01.c +++ b/testcases/kernel/syscalls/statvfs/statvfs01.c @@ -1,92 +1,63 @@ +// SPDX-License-Identifier: GPL-2.0 /* * Copyright (c) Wipro Technologies Ltd, 2005. All Rights Reserved. * AUTHOR: Prashant P Yendigeri <prashant.yendigeri@wipro.com> - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of version 2 of the GNU General Public License as - * published by the Free Software Foundation. - * - * This program is distributed in the hope that it would be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - * + * Copyright (c) 2022 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com> */ -/* - * DESCRIPTION - * This is a Phase I test for the statvfs(2) system call. - * It is intended to provide a limited exposure of the system call. - * This call behaves similar to statfs. + +/*\ + * [Description] + * + * Verify that statvfs() executes successfully for all + * available filesystems. Verify statvfs.f_namemax field + * by trying to create files of valid and invalid length names. */ -#include <stdio.h> -#include <unistd.h> -#include <errno.h> #include <sys/statvfs.h> -#include <stdint.h> - -#include "test.h" +#include "tst_test.h" -#define TEST_PATH "/" +#define MNT_POINT "mntpoint" +#define TEST_PATH MNT_POINT"/testfile" +#define NLS_MAX_CHARSET_SIZE 6 -static void setup(void); -static void cleanup(void); - -char *TCID = "statvfs01"; -int TST_TOTAL = 1; - -int main(int ac, char **av) +static void run(void) { struct statvfs buf; - int lc; - - tst_parse_opts(ac, av, NULL, NULL); - - setup(); + char *valid_fname, *toolong_fname; + long fs_type; - for (lc = 0; TEST_LOOPING(lc); lc++) { + fs_type = tst_fs_type(TEST_PATH); - tst_count = 0; + TST_EXP_PASS(statvfs(TEST_PATH, &buf)); - TEST(statvfs(TEST_PATH, &buf)); + toolong_fname = SAFE_MALLOC(buf.f_namemax + 1); + if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) + valid_fname = SAFE_MALLOC(buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); + else + valid_fname = SAFE_MALLOC(buf.f_namemax - 1); - if (TEST_RETURN == -1) { - tst_resm(TFAIL | TTERRNO, "statvfs(%s, ...) failed", - TEST_PATH); - } else { - tst_resm(TPASS, "statvfs(%s, ...) passed", TEST_PATH); - } + memset(toolong_fname, 'b', buf.f_namemax + 1); + if (fs_type == TST_VFAT_MAGIC || fs_type == TST_EXFAT_MAGIC) + memset(valid_fname, 'a', buf.f_namemax / NLS_MAX_CHARSET_SIZE - 1); + else + memset(valid_fname, 'a', buf.f_namemax - 1); - } + TST_EXP_FD(creat(valid_fname, 0444)); + SAFE_CLOSE(TST_RET); - tst_resm(TINFO, "This call is similar to statfs"); - tst_resm(TINFO, "Extracting info about the '%s' file system", - TEST_PATH); - tst_resm(TINFO, "file system block size = %lu bytes", buf.f_bsize); - tst_resm(TINFO, "file system fragment size = %lu bytes", buf.f_frsize); - tst_resm(TINFO, "file system free blocks = %ju", - (uintmax_t) buf.f_bfree); - tst_resm(TINFO, "file system total inodes = %ju", - (uintmax_t) buf.f_files); - tst_resm(TINFO, "file system free inodes = %ju", - (uintmax_t) buf.f_ffree); - tst_resm(TINFO, "file system id = %lu", buf.f_fsid); - tst_resm(TINFO, "file system max filename length = %lu", buf.f_namemax); - - cleanup(); - tst_exit(); + TST_EXP_FAIL(creat(toolong_fname, 0444), ENAMETOOLONG); } static void setup(void) { - tst_sig(NOFORK, DEF_HANDLER, cleanup); - - TEST_PAUSE; + SAFE_TOUCH(TEST_PATH, 0666, NULL); } -static void cleanup(void) -{ -} +static struct tst_test test = { + .test_all = run, + .setup = setup, + .needs_root = 1, + .mount_device = 1, + .mntpoint = MNT_POINT, + .all_filesystems = 1 +};
Removed the TINFO statements, Added a validation of statvfs.f_namemax field by trying to create files of valid and invalid length names. Changes in v5: Enabled file creation for vfat and exfat filesystems also. Signed-off-by: Avinesh Kumar <akumar@suse.de> --- testcases/kernel/syscalls/statvfs/statvfs01.c | 109 +++++++----------- 1 file changed, 40 insertions(+), 69 deletions(-)