[LTP] 回复: [PATCH 3/3] umount03: Simplify test using TST_ macros

[Yang Xu (Fujitsu)]

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
________________________________
发件人: Petr Vorel <pvorel@suse.cz>
发送时间: 2023年10月26日 8:05
收件人: Xu, Yang/徐 杨 <xuyang2018.jy@fujitsu.com>
抄送: ltp@lists.linux.it <ltp@lists.linux.it>
主题: Re: [LTP] [PATCH 3/3] umount03: Simplify test using TST_ macros

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