[LTP] [PATCH 1/1] syscalls: Add missing WEXITSTATUS() check

[LTP] [PATCH 1/1] syscalls: Add missing WEXITSTATUS() check

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
I haven't found any other test which would need it.
 testcases/kernel/syscalls/madvise/madvise08.c | 2 +-
 testcases/kernel/syscalls/select/select03.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
index 96bcaf159a..cd5ce722a5 100644
--- a/testcases/kernel/syscalls/madvise/madvise08.c
+++ b/testcases/kernel/syscalls/madvise/madvise08.c
@@ -173,7 +173,7 @@ static pid_t run_child(int advice)
 	SAFE_WAITPID(pid, &status, 0);
 	if (WIFSIGNALED(status) && WCOREDUMP(status))
 		return pid;
-	if (WIFEXITED(status))
+	if (WIFEXITED(status) && !WEXITSTATUS(status))
 		return 0;
 
 	tst_res(TCONF, "No coredump produced after signal (%d)",
diff --git a/testcases/kernel/syscalls/select/select03.c b/testcases/kernel/syscalls/select/select03.c
index 1cec3a4c76..216b22104f 100644
--- a/testcases/kernel/syscalls/select/select03.c
+++ b/testcases/kernel/syscalls/select/select03.c
@@ -77,7 +77,7 @@ static void run(unsigned int n)
 
 	SAFE_WAITPID(pid, &status, 0);
 
-	if (WIFEXITED(status))
+	if (WIFEXITED(status) && !WEXITSTATUS(status))
 		return;
 
 	if (tst_variant == GLIBC_SELECT_VARIANT &&
-- 
2.45.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] syscalls: Add missing WEXITSTATUS() check

[Cyril Hrubis]

Hi!
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> I haven't found any other test which would need it.
>  testcases/kernel/syscalls/madvise/madvise08.c | 2 +-
>  testcases/kernel/syscalls/select/select03.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
> index 96bcaf159a..cd5ce722a5 100644
> --- a/testcases/kernel/syscalls/madvise/madvise08.c
> +++ b/testcases/kernel/syscalls/madvise/madvise08.c
> @@ -173,7 +173,7 @@ static pid_t run_child(int advice)
>  	SAFE_WAITPID(pid, &status, 0);
>  	if (WIFSIGNALED(status) && WCOREDUMP(status))
>  		return pid;
> -	if (WIFEXITED(status))
> +	if (WIFEXITED(status) && !WEXITSTATUS(status))
>  		return 0;
>  
>  	tst_res(TCONF, "No coredump produced after signal (%d)",

This one is not complete. We do exit(1) if madvise fails that would be
previously caught here so we have to add:

diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
index 96bcaf159..f8f74ef4f 100644
--- a/testcases/kernel/syscalls/madvise/madvise08.c
+++ b/testcases/kernel/syscalls/madvise/madvise08.c
@@ -165,7 +165,7 @@ static pid_t run_child(int advice)
                                fmem,
                                FMEMSIZE,
                                advstr);
-                       exit(1);
+                       exit(0);
                }
                abort();
        }


> diff --git a/testcases/kernel/syscalls/select/select03.c b/testcases/kernel/syscalls/select/select03.c
> index 1cec3a4c76..216b22104f 100644
> --- a/testcases/kernel/syscalls/select/select03.c
> +++ b/testcases/kernel/syscalls/select/select03.c
> @@ -77,7 +77,7 @@ static void run(unsigned int n)
>  
>  	SAFE_WAITPID(pid, &status, 0);
>  
> -	if (WIFEXITED(status))
> +	if (WIFEXITED(status) && !WEXITSTATUS(status))
>  		return;
>  
>  	if (tst_variant == GLIBC_SELECT_VARIANT &&
> -- 
> 2.45.2
> 

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] syscalls: Add missing WEXITSTATUS() check

[Petr Vorel]

Hi Cyril,

> > +++ b/testcases/kernel/syscalls/madvise/madvise08.c
> > @@ -173,7 +173,7 @@ static pid_t run_child(int advice)
> >  	SAFE_WAITPID(pid, &status, 0);
> >  	if (WIFSIGNALED(status) && WCOREDUMP(status))
> >  		return pid;
> > -	if (WIFEXITED(status))
> > +	if (WIFEXITED(status) && !WEXITSTATUS(status))
> >  		return 0;

> >  	tst_res(TCONF, "No coredump produced after signal (%d)",

> This one is not complete. We do exit(1) if madvise fails that would be
> previously caught here so we have to add:

> diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
> index 96bcaf159..f8f74ef4f 100644
> --- a/testcases/kernel/syscalls/madvise/madvise08.c
> +++ b/testcases/kernel/syscalls/madvise/madvise08.c
> @@ -165,7 +165,7 @@ static pid_t run_child(int advice)
>                                 fmem,
>                                 FMEMSIZE,
>                                 advstr);
> -                       exit(1);
> +                       exit(0);
>                 }
>                 abort();
>         }

Good catch, thanks!

Maybe it'd be more logical to keep exit(1) (it's error) and check against
WEXITSTATUS(status) == 1, but it's a minor detail, let's keep exit(0).

Can I merge with your RBT?

Kind regards,
Petr


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] syscalls: Add missing WEXITSTATUS() check

[Cyril Hrubis]

Hi!
> > > -	if (WIFEXITED(status))
> > > +	if (WIFEXITED(status) && !WEXITSTATUS(status))
> > >  		return 0;
> 
> > >  	tst_res(TCONF, "No coredump produced after signal (%d)",
> 
> > This one is not complete. We do exit(1) if madvise fails that would be
> > previously caught here so we have to add:
> 
> > diff --git a/testcases/kernel/syscalls/madvise/madvise08.c b/testcases/kernel/syscalls/madvise/madvise08.c
> > index 96bcaf159..f8f74ef4f 100644
> > --- a/testcases/kernel/syscalls/madvise/madvise08.c
> > +++ b/testcases/kernel/syscalls/madvise/madvise08.c
> > @@ -165,7 +165,7 @@ static pid_t run_child(int advice)
> >                                 fmem,
> >                                 FMEMSIZE,
> >                                 advstr);
> > -                       exit(1);
> > +                       exit(0);
> >                 }
> >                 abort();
> >         }
> 
> Good catch, thanks!
> 
> Maybe it'd be more logical to keep exit(1) (it's error) and check against
> WEXITSTATUS(status) == 1, but it's a minor detail, let's keep exit(0).

I wouldn't do so, as it may confuse people into thinking that the return
value actually carries any information, which it does not since the
failure has been already reported. I would just stick to exit(0) which
makes it more clear that we just need to exit the process, nothing more.

> Can I merge with your RBT?

Yes, with the fix above.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] syscalls: Add missing WEXITSTATUS() check

[Petr Vorel]

Hi Cyril,

...
> > > --- a/testcases/kernel/syscalls/madvise/madvise08.c
> > > +++ b/testcases/kernel/syscalls/madvise/madvise08.c
> > > @@ -165,7 +165,7 @@ static pid_t run_child(int advice)
> > >                                 fmem,
> > >                                 FMEMSIZE,
> > >                                 advstr);
> > > -                       exit(1);
> > > +                       exit(0);
> > >                 }
> > >                 abort();
> > >         }

> > Good catch, thanks!

> > Maybe it'd be more logical to keep exit(1) (it's error) and check against
> > WEXITSTATUS(status) == 1, but it's a minor detail, let's keep exit(0).

> I wouldn't do so, as it may confuse people into thinking that the return
> value actually carries any information, which it does not since the
> failure has been already reported. I would just stick to exit(0) which
> makes it more clear that we just need to exit the process, nothing more.

This makes sense, kept it this way.

> > Can I merge with your RBT?

> Yes, with the fix above.

Thanks, merged.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp