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

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> > 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));

That one should stay, that is the overall exit value from the test.

There is one below that that should be replaced with exit(0).

> > 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?

Anything but the main library process would do exit(0).

> Other option could be to keep library as is and have a custm 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.

That just checks for valid exit status.

> > 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.

The change would allow us to implement things like tst_brk(TFAIL, ...)
and tst_brk(TPASS, ...) correctly. The problem is that the result from
children after tst_brk() is propagated as a return value. What we need
instead is to add an abort flag to the results structure.

-- 
Cyril Hrubis
chrubis@suse.cz

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