[LTP] [PATCH v1] Refactor pidns16 test using new LTP API
Cyril Hrubis
chrubis@suse.cz
Mon Aug 15 16:00:41 CEST 2022
Hi!
> > +static int counter;
> Shouldn't this be volatile? And maybe also sig_atomic_t instead of int (even
> it's the same)?
As long as we only modify the counter from within the signal handler
it's not required.
The probelm that sig_atomic_t fixes is to make sure that the load/store
is single CPU instruction so that we are sure that we don not jump into
the handler in the middle of a sequence of load/store instruction.
And the volatile "turns off" caching of the value, so again anything
that is accessed from both the handler and the rest of the code should
be volatile.
> static volatile sig_atomic_t counter;
>
> > -char *TCID = "pidns16";
> > -int TST_TOTAL = 3;
> > -
> > -void child_signal_handler(int sig, siginfo_t * si, void *unused)
> > +static void child_signal_handler(LTP_ATTRIBUTE_UNUSED int sig, siginfo_t *si, LTP_ATTRIBUTE_UNUSED void *unused)
> > {
> > - static int c = 1;
> > - pid_t expected_pid;
> > -
> > - /* Verifying from which process the signal handler is signalled */
> > + pid_t exp_pid;
>
> > - switch (c) {
> > - case 1:
> > - expected_pid = PARENT_PID;
> > + switch (counter) {
> > + case 0:
> > + exp_pid = 0;
> > break;
> > - case 2:
> > - expected_pid = CHILD_PID;
> > + case 1:
> > + exp_pid = 1;
> > break;
> > default:
> > - tst_resm(TBROK, "child should NOT be signalled 3+ times");
> > + tst_brk(TBROK, "Child should NOT be signalled 3+ times");
> > return;
> > }
>
> very nit: I'd use if (counter
> if (counter > 1) {
> tst_brk(TBROK, "Child should NOT be signalled 3+ times");
> return;
> }
> exp_pid = counter;
The bigger problem though is that we call tst_brk() and tst_res() in the
handler, which is not safe at all. The are couple things that can go
wrong here, mostly glibc locks can get deadlocked for the underlying
FILE* and if the underlying fprintf() calls malloc() the malloc data
structures may end up corrupted. It's quite rare for this to happen, but
there have been a few tests that were failing with small probability due
to these mistakes.
--
Cyril Hrubis
chrubis@suse.cz
More information about the ltp
mailing list