From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH] lib/tst_supported_fs_types.c: Add tmpfs to filesystem whitelist
Date: Mon, 8 Mar 2021 17:11:51 +0100 [thread overview]
Message-ID: <YEZMx0TF2FlipBiG@yuki.lan> (raw)
In-Reply-To: <20210304020838.259422-1-zhaogongyi@huawei.com>
Hi!
> @@ -34,6 +35,10 @@ static int has_mkfs(const char *fs_type)
>
> sprintf(buf, "mkfs.%s >/dev/null 2>&1", fs_type);
>
> + if (strstr(buf, "mkfs.tmpfs")) {
> + return 1;
> + }
The curly braces around single line statement are useless here.
Also it would be cleaner if we checked before the sprintf and against
the fs_type instead with if (!strcmp(fs_type, "tmpfs")).
We may also print a TINFO message something along the lines:
tst_res(TINFO, "mkfs is not needed for tmpfs");
> ret = tst_system(buf);
>
> if (WEXITSTATUS(ret) == 127) {
> @@ -50,17 +55,30 @@ static int has_kernel_support(const char *fs_type, int flags)
> static int fuse_supported = -1;
> const char *tmpdir = getenv("TMPDIR");
> char buf[128];
> + char template[PATH_MAX];
> int ret;
>
> if (!tmpdir)
> tmpdir = "/tmp";
>
> - mount("/dev/zero", tmpdir, fs_type, 0, NULL);
> - if (errno != ENODEV) {
> + sprintf(template, "%s/mountXXXXXX", tmpdir);
We should at least use snprintf() with sizeof(template) in order not to
overrun the buffer. Or we can use asprintf() instead and free the buffer
later on.
> + if (mkdtemp(template) == NULL) {
> + tst_res(TWARN | TERRNO , "%s: mkdtemp(%s) failed", __func__, template);
> + }
I guess that we can tst_brk(TBROK, ""); here if mkdtemp() failed,
continuing here we would pass non-existing directory thte mount()
syscall below.
Also please do not include the __func__ in the message here.
> + ret = mount("/dev/zero", template, fs_type, 0, NULL);
> + if ((ret && errno != ENODEV) || !ret) {
> + if (!ret)
> + tst_umount(template);
> tst_res(TINFO, "Kernel supports %s", fs_type);
> + if (rmdir(template) == -1)
> + tst_res(TWARN | TERRNO, "rmdir %s failed", template);
We do have SAFE_RMDIR() please use that.
> return 1;
> }
>
> + if (rmdir(template) == -1)
> + tst_res(TWARN | TERRNO, "rmdir %s failed", template);
Here as well.
> /* Is FUSE supported by kernel? */
> if (fuse_supported == -1) {
> ret = open("/dev/fuse", O_RDWR);
> --
> 2.17.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Cyril Hrubis
chrubis@suse.cz
next prev parent reply other threads:[~2021-03-08 16:11 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-04 2:08 [LTP] [PATCH] lib/tst_supported_fs_types.c: Add tmpfs to filesystem whitelist Zhao Gongyi
2021-03-08 16:11 ` Cyril Hrubis [this message]
-- strict thread matches above, loose matches on Subject: below --
2021-03-09 13:08 zhaogongyi
2021-03-09 13:01 Zhao Gongyi
2021-03-10 12:29 ` Cyril Hrubis
2021-03-12 10:46 ` Petr Vorel
2021-04-13 17:28 ` Cyril Hrubis
2021-03-04 2:14 zhaogongyi
2021-02-27 9:33 zhaogongyi
2021-03-02 10:43 ` Cyril Hrubis
2021-02-26 9:06 Zhao Gongyi
2021-02-26 9:54 ` Cyril Hrubis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YEZMx0TF2FlipBiG@yuki.lan \
--to=chrubis@suse.cz \
--cc=ltp@lists.linux.it \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox