Re: [LTP] [PATCH] [RFC] lib/tst_test.c: Fix tst_brk() handling

[Petr Vorel <pvorel@suse.cz>]

Hi all,

new approach looks as an improvement to me.

So you plan to send v2 to send sigkill signal to the test process group in
the heartbeat handler?

> On Tue, Nov 26, 2024 at 10:50 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> > Hi!
> > > > This makes the tst_brk() handling cleaner and saner as instead of
> > > > propagating the tst_brk() result in a return value an abort flag is
> > > > introduced into the shared memory.

> > > > Now:

> > > > - All the processes but the library one that reports the results exit
> > > >   with 0
> > > > - tst_brk(TBROK, ...) increments result conters, sets the abort flag.
> > > >   and exit current process
> > > > - all other tst_brk() variants will just increments the countes and
> > > >   exits the current process

> > > It removes the easy way for parent to check that child hasn't run into
> > > any issues,
> > > but I can't recall a specific test we have today that depends on it.

> > I suppose that we can make the tst_brk flag part of a public API if
> > anyone needs that but I guess that in the case of tst_brk(TBROK, ...)
> > all we want is to make the test processes exit as soon as possible.

> > > > This makes the tst_brk() behavior well defined so we can now even call
> > > > tst_brk() with TFAIL and TPASS as well.

> > > What's the use-case for it? Wouldn't it be more clear to just report
> > > TPASS + exit?

> > I think this makes actually the API more consistent. I.e. tst_res()
> > reports result and tst_brk() reports result and exits the current
> > process.

+1

> > I think that we all carry a mental baggage that associates the tst_brk()
> > call with an error, but that is something we forced upon ourselves.
> > AFAIK it's short for tst_break, which itself only suggests that it does
> > exit the current process, similar to break being used in switch()
> > statement.

+1.

> Indeed. With this change, we don't have to use "tst_res() + return" for
> some situations,
> only tst_brk(TPASS, ) would be enough.

+1. We have tst_brk(TFAIL) anyway. We should also unify shell API.

nit (commenting the implementation):

+/*
Don't we want to have it in the docs, e.g. /** + document parameters?
+ * Prints results, cleans up after the test library and exits the test library

+++ b/lib/newlib_tests/test23.c
very nit: I would prefer short description names for tests than numbers.

Kind regards,
Petr

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