public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 1/1] syscalls/fsmount01: Add test for new mount API v5.2
Date: Fri, 7 Feb 2020 15:04:46 +0100	[thread overview]
Message-ID: <20200207140404.GA8492@dell5510> (raw)
In-Reply-To: <20200207132001.GC19508@rei.lan>

Hi,

> > +	while (fgets(line, LINELENGTH, file) != NULL) {
> > +		if (strstr(line, mntpoint) != NULL) {

> It's not necessary to check against NULL, we can simply write these as:

> 	while (fgets(line, sizeof(line), file)) {
> 		...

> > +			ret = 1;
> > +			break;
> > +		}
> > +	}
> > +	SAFE_FCLOSE(file);
> > +	return ret;
> > +}
> > +
> > +static void cleanup(void)
> > +{
> > +	if (is_mounted) {
> > +		TEST(tst_umount(MNTPOINT));
> > +		if (TST_RET != 0)
> > +			tst_brk(TBROK | TTERRNO, "umount failed in cleanup");

> I do remmeber commenting this that the tst_umount() already prints a
> WARN which will fail the test anyways, so there is no point in handling
> the return from tst_umount() here.
I guess SAFE_UMOUNT(MNTPOINT) will be the best.

...
> > +	if (ismount(MNTPOINT)) {
> > +		tst_res(TPASS, "new mount API from v5.2 works");
> > +		TEST(tst_umount(MNTPOINT));
> > +		if (TST_RET != 0)
> > +			tst_brk(TBROK | TTERRNO, "umount failed");

> And here as well.

> Apart from these two minor comments the test looks good, you can add my
> Reviewed-by.

Thanks for review, sorry for not catching these errors.
Going to fix it with your Reviewed-by, with these fixes.

Kind regards,
Petr

diff --git testcases/kernel/syscalls/fsmount/fsmount01.c testcases/kernel/syscalls/fsmount/fsmount01.c
index 7ba50753f..64e8c4744 100644
--- testcases/kernel/syscalls/fsmount/fsmount01.c
+++ testcases/kernel/syscalls/fsmount/fsmount01.c
@@ -25,7 +25,7 @@ static int ismount(char *mntpoint)
 
 	file = SAFE_FOPEN("/proc/mounts", "r");
 
-	while (fgets(line, LINELENGTH, file) != NULL) {
+	while (fgets(line, sizeof(line), file)) {
 		if (strstr(line, mntpoint) != NULL) {
 			ret = 1;
 			break;
@@ -37,11 +37,8 @@ static int ismount(char *mntpoint)
 
 static void cleanup(void)
 {
-	if (is_mounted) {
-		TEST(tst_umount(MNTPOINT));
-		if (TST_RET != 0)
-			tst_brk(TBROK | TTERRNO, "umount failed in cleanup");
-	}
+	if (is_mounted)
+		SAFE_UMOUNT(MNTPOINT)
 }
 
 static void test_newmount(void)
@@ -80,9 +77,7 @@ static void test_newmount(void)
 
 	if (ismount(MNTPOINT)) {
 		tst_res(TPASS, "new mount API from v5.2 works");
-		TEST(tst_umount(MNTPOINT));
-		if (TST_RET != 0)
-			tst_brk(TBROK | TTERRNO, "umount failed");
+		SAFE_UMOUNT(MNTPOINT)
 		is_mounted = 0;
 	} else
 		tst_res(TFAIL, "new mount API from v5.2 works");

  reply	other threads:[~2020-02-07 14:04 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-06 16:27 [LTP] [PATCH 1/1] syscalls/fsmount01: Add test for new mount API v5.2 Petr Vorel
2020-02-07  8:18 ` Zorro Lang
2020-02-07  9:05   ` Petr Vorel
2020-02-07 13:20 ` Cyril Hrubis
2020-02-07 14:04   ` Petr Vorel [this message]
2020-02-07 14:08     ` 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=20200207140404.GA8492@dell5510 \
    --to=pvorel@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