All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.