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

[Petr Vorel <pvorel@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).

Ah, thx for info. Makes sense to me.

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

OK. Shouldn't be here TFAIL as well (or even TWARN)?

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

Ah, now I see why you want to do it properly. Makes sense.

Kind regards,
Petr

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