[LTP] [PATCH v1 2/2] lib: moves test infrastructure states into a shared context structure

[Li Wang]

On Wed, Jun 4, 2025 at 7:55 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> >       if (tst_test->needs_checkpoints) {
> > -             tst_futexes = (char *)results + sizeof(struct results);
> > -             tst_max_futexes = (size - sizeof(struct
> results))/sizeof(futex_t);
> > +             tst_futexes = ipc->futexes;
> > +
> > +             size_t futexes_offset = (char *)ipc->futexes - (char *)ipc;
> > +             tst_max_futexes = (size - futexes_offset) /
> sizeof(futex_t);
>                                            ^
>                                            This would be better as:
>                                            offsetof(struct ipc, futexes)
>

offsetof(struct ipc, futexes) returns the offset of the field futexes within
the struct ipc_region layout in memory. It does not reflect how many
futexes can be stored.

I guess you were hoping:

--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -152,7 +152,7 @@ static void setup_ipc(void)
        if (tst_test->needs_checkpoints) {
                tst_futexes = ipc->futexes;

-               size_t futexes_offset = (char *)ipc->futexes - (char *)ipc;
+               size_t futexes_offset = offsetof(struct ipc_region,
futexes);
                tst_max_futexes = (size - futexes_offset) / sizeof(futex_t);
        }

@@ -208,7 +208,7 @@ void tst_reinit(void)
        results = &ipc->results;

        tst_futexes = ipc->futexes;
-       size_t futexes_offset = (char *)ipc->futexes - (char *)ipc;
+       size_t futexes_offset = offsetof(struct ipc_region, futexes);
        tst_max_futexes = (size - futexes_offset) / sizeof(futex_t);

        if (context->tdebug)



> >       switch (ttype) {
> >       case TCONF:
> > -             tst_atomic_inc(&results->skipped);
> > +             tst_atomic_inc((int *)&results->skipped);
> >       break;
> >       case TPASS:
> > -             tst_atomic_inc(&results->passed);
> > +             tst_atomic_inc((int *)&results->passed);
> >       break;
> >       case TWARN:
> > -             tst_atomic_inc(&results->warnings);
> > +             tst_atomic_inc((int *)&results->warnings);
> >       break;
> >       case TFAIL:
> > -             tst_atomic_inc(&results->failed);
> > +             tst_atomic_inc((int *)&results->failed);
> >       break;
> >       case TBROK:
> > -             tst_atomic_inc(&results->broken);
> > +             tst_atomic_inc((int *)&results->broken);
> >       break;
>
> This gets ugly. I guess that it would be better to keep the results as
> int unless we change the tst_atomic.h to work with int32_t.
>
> Maybe we can actually drop the assembly fallbacks from tst_atomic.h
> since as far as I can tell the __atomic_*() functions were added to
> gcc-4.7 and the __sync_*() function were added into gcc-4.1 so unless we
> need to support compiler older than 4.1 we can drop the assembly and
> easily add support for atomic operations for int32_t.
>

That sounds good, I will look to see if we can refine the tst_atomic.h.


-- 
Regards,
Li Wang