[PATCH 2/2] printrusage: Correctly print the long types from the rusage struct

[Alistair Francis]

On Thu, Mar 5, 2020 at 5:05 PM Dmitry V. Levin <ldv at altlinux.org> wrote:
>
> On Wed, Mar 04, 2020 at 09:25:20PM -0800, Alistair Francis wrote:
> > On Wed, Mar 4, 2020 at 6:04 PM Dmitry V. Levin wrote:
> > > On Wed, Mar 04, 2020 at 05:30:34PM -0800, 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.
> > > >
> > > > There are two possible options here:
> > > >  1. Use the glibc getrusage() function. Which takes a struct rusage with
> > > >     a 64-bit time_t but does a syscall with a 32-bit time_t and handles
> > > >     the conversion internally.
> > > >  2. Specify our own rusage struct that uses a long for the time.
> > > >
> > > > This patch fixes the failure useing option 2 above as we then get to
> > > > call the raw syscall.
> > > >
> > > > Signed-off-by: Alistair Francis <alistair.francis at wdc.com>
> > > > ---
> > > >  print_timeval.c   | 16 ++++++++++++++++
> > > >  printrusage.c     |  4 ++--
> > > >  tests/getrusage.c | 28 ++++++++++++++++++++++++++--
> > > >  3 files changed, 44 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/print_timeval.c b/print_timeval.c
> > > > index ff87b27b..36bc843e 100644
> > > > --- a/print_timeval.c
> > > > +++ b/print_timeval.c
> > > > @@ -11,6 +11,10 @@
> > > >  #include DEF_MPERS_TYPE(timeval_t)
> > > >
> > > >  typedef struct timeval timeval_t;
> > > > +typedef struct old_timeval_t {
> > > > +     long tv_sec;
> > > > +     long tv_usec;
> > > > +} old_timeval_t;
> > >
> > > Sorry but this definition doesn't match the definition of struct
> > > __kernel_old_timeval in include/uapi/linux/time_types.h and
> > > arch/sparc/include/uapi/asm/posix_types.h files.
> >
> > Do you mean it should be __kernel_long_t [1]? I can change it to
> > __kernel_long_t .
> >
> > I'm not sure what to do to handle the space difference [2] though. I
> > think it ends up being long but I don't see a nice way to handle it.
>
> It definitely has to be __kernel_long_t on everything but sparc where
> tv_usec has to be int if I read arch/sparc/include/uapi/asm/posix_types.h
> correctly.
>
> BTW, since include/uapi/linux/time.h along with
> include/uapi/asm-generic/posix_types.h define struct timeval as
> essentially the same structure as struct __kernel_old_timeval,
> why do we need a separate printer for old_timeval_t?
> Is it due to libc that may define its struct timeval in an incompatible way?

Yes, we end up using the libc struct timeval (which is not the same as
__kernel_old_timeval).

I can actually just use __kernel_old_timeval directly and that works.

> Shouldn't timeval_t printers be fixed instead?
>
> > Would you prefer the fist option mentioned in my commit message of
> > just using the glibc call?
>
> Are you talking about the test?  I'd prefer to use libc functions
> if they allowed to test the parser thoroughly, which is unlikely.
> Maybe the test has to use libc functions for basic tests
> and direct syscall invocations for corner case tests.

I figured it was something like that, I'll keep the direct syscalls in
then, just wanted to check.

Alistair

>
>
> --
> ldv