Re: [LTP] [PATCH 1/1] select03: Fix false positive on TCONF

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril,

> Hi!
> > ---
> > Alternatively, we could revert to previous state (remove
> > "!WEXITSTATUS(status)" check), if we really don't care about any other
> > exit code.

> > Kind regards,
> > Petr

> >  testcases/kernel/syscalls/select/select03.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)

> > diff --git a/testcases/kernel/syscalls/select/select03.c b/testcases/kernel/syscalls/select/select03.c
> > index 216b22104f..34aea12603 100644
> > --- a/testcases/kernel/syscalls/select/select03.c
> > +++ b/testcases/kernel/syscalls/select/select03.c
> > @@ -77,8 +77,10 @@ static void run(unsigned int n)

> >  	SAFE_WAITPID(pid, &status, 0);

> > -	if (WIFEXITED(status) && !WEXITSTATUS(status))
> > +	if (WIFEXITED(status) &&
> > +		(WEXITSTATUS(status) == 0 || WEXITSTATUS(status) == TCONF)) {
> >  		return;

> I think that the main mistake here is that I kept the code in
> tst_vbrk_() that exits the test with a return value in the case of a
> child processes.

I suppose you talk about in tst_vbrk_ (lib/tst_test.c):

	if (getpid() == lib_pid)
		do_exit(TTYPE_RESULT(ttype));

> So ideal fix would be to change the test library not to
> do that,

So would you just do_exit(0) or even just exit(0)?
Or exit 0 for TPASS or TCONF and 1 otherwise (TBROK, TFAIL, TWARN)? Or what
exactly did you meant?

Other option could be to keep library as is and have a custom functions which
"translate" LTP exit codes. I was thinking something like this:

bool tst_exit_status(int status)
{
	if (!WEXITSTATUS(status))
		return false;

	if (WEXITSTATUS(status) == TPASS || WEXITSTATUS(status) == TCONF)
		return true;

	return false;
}

But we actually have check_child_status(pid_t pid, int status) for this:

static void check_child_status(pid_t pid, int status)
{
	int ret;

	if (WIFSIGNALED(status)) {
		tst_brk(TBROK, "Child (%i) killed by signal %s", pid,
			tst_strsig(WTERMSIG(status)));
	}

	if (!(WIFEXITED(status)))
		tst_brk(TBROK, "Child (%i) exited abnormally", pid);

	ret = WEXITSTATUS(status);
	switch (ret) {
	case TPASS:
	case TBROK:
	case TCONF:
	break;
	default:
		tst_brk(TBROK, "Invalid child (%i) exit value %i", pid, ret);
	}
}

BTW I'm surprised that there is TBROK whitelisted.

> but that would require a bit more work, especially we would
> have to look at all WAITPID() usages in newlib tests and make sure that
> there are not tests that depends on such behavior. If there are none it
> would stil require a few changes in the test library.

Well, what I fear more is to broke even more tests :). But if you consider is
useful enough I can do the cleanup.

Kind regards,
Petr

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