diff mbox series

[3/3] umount03: Simplify test using TST_ macros

Message ID 1697100058-2859-3-git-send-email-xuyang2018.jy@fujitsu.com
State Changes Requested
Headers show
Series [1/3] umount01: Simplify test using TST_ macros | expand

Commit Message

Yang Xu \(Fujitsu\) Oct. 12, 2023, 8:40 a.m. UTC
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/syscalls/umount/umount03.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Comments

Petr Vorel Oct. 26, 2023, 12:05 a.m. UTC | #1
Hi Xu,

> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  testcases/kernel/syscalls/umount/umount03.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)

this is not needed:
#include <errno.h>

This is for all 3 patches.

> diff --git a/testcases/kernel/syscalls/umount/umount03.c b/testcases/kernel/syscalls/umount/umount03.c
> index 1cef06fa1..e6bb523b4 100644
> --- a/testcases/kernel/syscalls/umount/umount03.c
> +++ b/testcases/kernel/syscalls/umount/umount03.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
> + * Copyright (c) Linux Test Project, 2003-2023
>   * Author: Nirmala Devi Dhanasekar <nirmala.devi@wipro.com>
>   *
>   * Verify that umount(2) returns -1 and sets errno to  EPERM if the user
> @@ -20,19 +21,12 @@ static int mount_flag;

>  static void verify_umount(void)
>  {
> -	TEST(umount(MNTPOINT));
> -
> -	if (TST_RET != -1) {
> -		tst_res(TFAIL, "umount() succeeds unexpectedly");
> -		return;
> -	}
> +	TST_EXP_FAIL(umount(MNTPOINT), EPERM, "umount(%s) Failed", MNTPOINT);
nit: I would prefer just:

	TST_EXP_FAIL(umount(MNTPOINT), EPERM);


>  	if (TST_ERR != EPERM) {
>  		tst_res(TFAIL | TTERRNO, "umount() should fail with EPERM");
>  		return;
>  	}
This should have been removed, it's redundant when TST_EXP_FAIL() is done.
> -
> -	tst_res(TPASS | TTERRNO, "umount() fails as expected");
>  }

>  static void setup(void)

With <errno.h> and if (TST_ERR != EPERM) removed you can add:
Reviewed-by: Petr Vorel <pvorel@suse.cz>

It would be good (as a separate commit) to reword the documentation and convert
it to docparse. Feel free to do it, or please let me know if I should do it.

Kind regards,
Petr
Petr Vorel Oct. 26, 2023, 12:06 a.m. UTC | #2
Hi Xu,


> this is not needed:
> #include <errno.h>

Also these aren't needed:
#include <sys/types.h>
#include <unistd.h>

> This is for all 3 patches.

Kind regards,
Petr
Yang Xu \(Fujitsu\) Oct. 26, 2023, 6:05 a.m. UTC | #3
Hi Petr,

>Hi Xu,

>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>> ---
>>  testcases/kernel/syscalls/umount/umount03.c | 10 ++--------
>>  1 file changed, 2 insertions(+), 8 deletions(-)

>this is not needed:
>#include <errno.h>

>This is for all 3 patches.

Yes.

>> diff --git a/testcases/kernel/syscalls/umount/umount03.c b/testcases/kernel/syscalls/umount/umount03.c
>> index 1cef06fa1..e6bb523b4 100644
>> --- a/testcases/kernel/syscalls/umount/umount03.c
>> +++ b/testcases/kernel/syscalls/umount/umount03.c
>> @@ -1,6 +1,7 @@
>>  // SPDX-License-Identifier: GPL-2.0-or-later
>>  /*
>>   * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
>> + * Copyright (c) Linux Test Project, 2003-2023
>>   * Author: Nirmala Devi Dhanasekar <nirmala.devi@wipro.com>
>>   *
>>   * Verify that umount(2) returns -1 and sets errno to  EPERM if the user
>> @@ -20,19 +21,12 @@ static int mount_flag;

>>  static void verify_umount(void)
>>  {
>> -     TEST(umount(MNTPOINT));
>> -
>> -     if (TST_RET != -1) {
>> -             tst_res(TFAIL, "umount() succeeds unexpectedly");
>> -             return;
>> -     }
>> +     TST_EXP_FAIL(umount(MNTPOINT), EPERM, "umount(%s) Failed", MNTPOINT);
>nit: I would prefer just:

>        TST_EXP_FAIL(umount(MNTPOINT), EPERM);

OK.

>>        if (TST_ERR != EPERM) {
>>                tst_res(TFAIL | TTERRNO, "umount() should fail with EPERM");
>>                return;
>>        }
>This should have been removed, it's redundant when TST_EXP_FAIL() is done.

Yes.

>> -
>> -     tst_res(TPASS | TTERRNO, "umount() fails as expected");
>>  }

>>  static void setup(void)

>With <errno.h> and if (TST_ERR != EPERM) removed you can add:
>Reviewed-by: Petr Vorel <pvorel@suse.cz>

>It would be good (as a separate commit) to reword the documentation and convert
>it to docparse. Feel free to do it, or please let me know if I should do it.

Thanks for all your suggestions, I will correct it in v2 version.

I see you have already merge this 584b87a<https://github.com/linux-test-project/ltp/commit/584b87ae94d61d5a3cfc47c9146af7b778e2c9ba>. I will also rewrite the documentation for umount01/03
as a separate commit.

Best Regards
Yang Xu

>Kind regards,
>Petr
Yang Xu \(Fujitsu\) Oct. 26, 2023, 6:07 a.m. UTC | #4
Hi Petr,

>Hi Xu,


>> this is not needed:
>> #include <errno.h>

>Also these aren't needed:
>#include <sys/types.h>
>#include <unistd.h>

Yes.

>> This is for all 3 patches.

>Kind regards,
>Petr

Best Regards
Yang Xu
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/umount/umount03.c b/testcases/kernel/syscalls/umount/umount03.c
index 1cef06fa1..e6bb523b4 100644
--- a/testcases/kernel/syscalls/umount/umount03.c
+++ b/testcases/kernel/syscalls/umount/umount03.c
@@ -1,6 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
+ * Copyright (c) Linux Test Project, 2003-2023
  * Author: Nirmala Devi Dhanasekar <nirmala.devi@wipro.com>
  *
  * Verify that umount(2) returns -1 and sets errno to  EPERM if the user
@@ -20,19 +21,12 @@  static int mount_flag;
 
 static void verify_umount(void)
 {
-	TEST(umount(MNTPOINT));
-
-	if (TST_RET != -1) {
-		tst_res(TFAIL, "umount() succeeds unexpectedly");
-		return;
-	}
+	TST_EXP_FAIL(umount(MNTPOINT), EPERM, "umount(%s) Failed", MNTPOINT);
 
 	if (TST_ERR != EPERM) {
 		tst_res(TFAIL | TTERRNO, "umount() should fail with EPERM");
 		return;
 	}
-
-	tst_res(TPASS | TTERRNO, "umount() fails as expected");
 }
 
 static void setup(void)