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

[Dmitry V. Levin]

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?
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.


-- 
ldv