[PATCH v6 03/13] tests/waitid: Correctly use the long types from the rusage struct

Alistair Francis alistair23 at gmail.com
Wed Apr 1 17:41:12 UTC 2020


On Tue, Mar 31, 2020 at 4:45 PM Dmitry V. Levin <ldv at altlinux.org> wrote:
>
> On Fri, Mar 20, 2020 at 03:09:29PM -0700, Alistair Francis wrote:
> > The kernel's rusage struct uses the kernel's __kernel_old_timeval which
> > means that even for 32-bit archs with 64-bit time_t (like RV32) the time
> > values are 32-bit.
> >
> > We can fix this by making sure we don't use libc's rusage struct and
> > instead use our own with the kernel's old timeval.
> >
> > Signed-off-by: Alistair Francis <alistair.francis at wdc.com>
> > ---
> >  tests/waitid.c | 108 +++++++++----------------------------------------
> >  1 file changed, 18 insertions(+), 90 deletions(-)
> >
> > diff --git a/tests/waitid.c b/tests/waitid.c
> > index 71c10f3c..52406d13 100644
> > --- a/tests/waitid.c
> > +++ b/tests/waitid.c
> > @@ -15,83 +15,11 @@
> >  #include <string.h>
> >  #include <unistd.h>
> >  #include <sys/wait.h>
> > -#include <sys/resource.h>
> >  #include "scno.h"
> > -
> > -/* Workaround for glibc/kernel interface discrepancy */
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_MAXRSS_WORD
> > -# define RU_MAXRSS __ru_maxrss_word
> > -#else
> > -# define RU_MAXRSS ru_maxrss
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_IXRSS_WORD
> > -# define RU_IXRSS __ru_ixrss_word
> > -#else
> > -# define RU_IXRSS ru_ixrss
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_IDRSS_WORD
> > -# define RU_IDRSS __ru_idrss_word
> > -#else
> > -# define RU_IDRSS ru_idrss
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_ISRSS_WORD
> > -# define RU_ISRSS __ru_isrss_word
> > -#else
> > -# define RU_ISRSS ru_isrss
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_MINFLT_WORD
> > -# define RU_MINFLT __ru_minflt_word
> > -#else
> > -# define RU_MINFLT ru_minflt
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_MAJFLT_WORD
> > -# define RU_MAJFLT __ru_majflt_word
> > -#else
> > -# define RU_MAJFLT ru_majflt
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_NSWAP_WORD
> > -# define RU_NSWAP __ru_nswap_word
> > -#else
> > -# define RU_NSWAP ru_nswap
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_INBLOCK_WORD
> > -# define RU_INBLOCK __ru_inblock_word
> > -#else
> > -# define RU_INBLOCK ru_inblock
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_OUBLOCK_WORD
> > -# define RU_OUBLOCK __ru_oublock_word
> > -#else
> > -# define RU_OUBLOCK ru_oublock
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_MSGSND_WORD
> > -# define RU_MSGSND __ru_msgsnd_word
> > -#else
> > -# define RU_MSGSND ru_msgsnd
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_MSGRCV_WORD
> > -# define RU_MSGRCV __ru_msgrcv_word
> > -#else
> > -# define RU_MSGRCV ru_msgrcv
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_NSIGNALS_WORD
> > -# define RU_NSIGNALS __ru_nsignals_word
> > -#else
> > -# define RU_NSIGNALS ru_nsignals
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_NVCSW_WORD
> > -# define RU_NVCSW __ru_nvcsw_word
> > -#else
> > -# define RU_NVCSW ru_nvcsw
> > -#endif
> > -#ifdef HAVE_STRUCT_RUSAGE___RU_NIVCSW_WORD
> > -# define RU_NIVCSW __ru_nivcsw_word
> > -#else
> > -# define RU_NIVCSW ru_nivcsw
> > -#endif
> > +#include "kernel_rusage.h"
> >
> >  static const char *
> > -sprint_rusage(const struct rusage *const ru)
> > +sprint_rusage(const rusage_t *const ru)
> >  {
> >       static char buf[1024];
> >       snprintf(buf, sizeof(buf),
> > @@ -120,20 +48,20 @@ sprint_rusage(const struct rusage *const ru)
> >                , (long long) ru->ru_stime.tv_sec
> >                , zero_extend_signed_to_ull(ru->ru_stime.tv_usec)
> >  #if VERBOSE
> > -              , zero_extend_signed_to_ull(ru->RU_MAXRSS)
> > -              , zero_extend_signed_to_ull(ru->RU_IXRSS)
> > -              , zero_extend_signed_to_ull(ru->RU_IDRSS)
> > -              , zero_extend_signed_to_ull(ru->RU_ISRSS)
> > -              , zero_extend_signed_to_ull(ru->RU_MINFLT)
> > -              , zero_extend_signed_to_ull(ru->RU_MAJFLT)
> > -              , zero_extend_signed_to_ull(ru->RU_NSWAP)
> > -              , zero_extend_signed_to_ull(ru->RU_INBLOCK)
> > -              , zero_extend_signed_to_ull(ru->RU_OUBLOCK)
> > -              , zero_extend_signed_to_ull(ru->RU_MSGSND)
> > -              , zero_extend_signed_to_ull(ru->RU_MSGRCV)
> > -              , zero_extend_signed_to_ull(ru->RU_NSIGNALS)
> > -              , zero_extend_signed_to_ull(ru->RU_NVCSW)
> > -              , zero_extend_signed_to_ull(ru->RU_NIVCSW)
> > +              , zero_extend_signed_to_ull(ru->ru_maxrss)
> > +              , zero_extend_signed_to_ull(ru->ru_ixrss)
> > +              , zero_extend_signed_to_ull(ru->ru_idrss)
> > +              , zero_extend_signed_to_ull(ru->ru_isrss)
> > +              , zero_extend_signed_to_ull(ru->ru_minflt)
> > +              , zero_extend_signed_to_ull(ru->ru_majflt)
> > +              , zero_extend_signed_to_ull(ru->ru_nswap)
> > +              , zero_extend_signed_to_ull(ru->ru_inblock)
> > +              , zero_extend_signed_to_ull(ru->ru_oublock)
> > +              , zero_extend_signed_to_ull(ru->ru_msgsnd)
> > +              , zero_extend_signed_to_ull(ru->ru_msgrcv)
> > +              , zero_extend_signed_to_ull(ru->ru_nsignals)
> > +              , zero_extend_signed_to_ull(ru->ru_nvcsw)
> > +              , zero_extend_signed_to_ull(ru->ru_nivcsw)
> >  #endif
> >                );
> >       return buf;
> > @@ -200,7 +128,7 @@ do_waitid(const unsigned int idtype,
> >         const unsigned int id,
> >         const siginfo_t *const infop,
> >         const unsigned int options,
> > -       const struct rusage *const rusage)
> > +       const rusage_t *const rusage)
> >  {
> >       sigset_t mask = {};
> >       sigaddset(&mask, SIGCHLD);
> > @@ -241,7 +169,7 @@ main(void)
> >
> >       TAIL_ALLOC_OBJECT_CONST_PTR(siginfo_t, sinfo);
> >       memset(sinfo, 0, sizeof(*sinfo));
> > -     TAIL_ALLOC_OBJECT_CONST_PTR(struct rusage, rusage);
> > +     TAIL_ALLOC_OBJECT_CONST_PTR(rusage_t, rusage);
> >       if (do_waitid(P_PID, pid, sinfo, WNOHANG|WEXITED|WSTOPPED, rusage))
> >               perror_msg_and_fail("waitid #2");
> >       tprintf("waitid(P_PID, %d, {}, WNOHANG|WEXITED|WSTOPPED, %s) = 0\n",
> > --
> > 2.25.1
>
> This looks mostly OK, but in line with my previous comment please rename
> rusage_t to kernel_rusage_t.

Fixed.

>
> Also, the configure.ac part of commit
> 94d56c9c30ec152ac31d41ea0a1ea82b41f6b1eb is no longer used,
> it would make sense to remove it along with this change.

Ok, removed.

Alistair

>
>
> --
> ldv


More information about the Strace-devel mailing list