Re: [PATCH v3 3/7] y2038: alpha: Rename valid_timeval_to_timeval64 to valid_timeval32_to_timeval

[Lukasz Majewski <lukma at denx dot de>]

Hi Adhemerval,

> On 29/01/2020 09:59, Lukasz Majewski wrote:
> > Without this patch the naming convention for functions to convert
> > struct timeval32 to struct timeval (which supports 64 bit time on
> > Alpha) was a bit misleading. The name 'valid_timeval_to_timeval64'
> > suggest conversion of struct timeval to struct __timeval64 (as in
> > ./include/time.h).
> > 
> > As on alpha the struct timeval supports 64 bit time it seems more
> > readable to emphasis struct timeval32 in the conversion function
> > name.
> > 
> > Hence the helper function naming change to
> > 'valid_timeval32_to_timeval'.
> > 
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
> > 
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>  
> 
> Usually I don't see much gain in such changes. It the current name 
> clashing with a new proposed internal symbol? 

The problem here is that the name is a bit misleading, as alpha's
struct timeval has 64 bit tv.sec (alpha generally supports 64 bit time).

The rename here is to emphasis that we do convert "natural" (for alpha)
struct timeval to struct timeval32 (which is needed when passing the
pointer to syscalls).

In that way the 'valid_timeval_to_timeval64' can follow the pattern,
which we do have now in ./include/time.h.

(it is just to make the naming convention more consistent)

Just to point out - Alistair in his patch set (prepared on top of this
one):
https://patchwork.ozlabs.org/patch/1232967/

took one step further and added "alpha_" prefix to those names
(converted in this patch).

> 
> Anyway, it seems fine though.
> 
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> 
> > 
> > ---
> > Changes for v3:
> > - None
> > 
> > Changes for v2:
> > - None
> > ---
> >  sysdeps/unix/sysv/linux/alpha/osf_adjtime.c   | 4 ++--
> >  sysdeps/unix/sysv/linux/alpha/osf_setitimer.c | 4 ++--
> >  sysdeps/unix/sysv/linux/alpha/osf_utimes.c    | 4 ++--
> >  sysdeps/unix/sysv/linux/alpha/tv32-compat.h   | 2 +-
> >  4 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c
> > b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c index
> > cd864686f6..5ac72e252f 100644 ---
> > a/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c +++
> > b/sysdeps/unix/sysv/linux/alpha/osf_adjtime.c @@ -57,7 +57,7 @@ int
> >  attribute_compat_text_section
> >  __adjtime_tv32 (const struct timeval32 *itv, struct timeval32 *otv)
> >  {
> > -  struct timeval itv64 = valid_timeval_to_timeval64 (*itv);
> > +  struct timeval itv64 = valid_timeval32_to_timeval (*itv);
> >    struct timeval otv64;
> >  
> >    if (__adjtime (&itv64, &otv64) == -1)
> > @@ -91,7 +91,7 @@ __adjtimex_tv32 (struct timex32 *tx)
> >    tx64.calcnt    = tx->calcnt;
> >    tx64.errcnt    = tx->errcnt;
> >    tx64.stbcnt    = tx->stbcnt;
> > -  tx64.time      = valid_timeval_to_timeval64 (tx->time);
> > +  tx64.time      = valid_timeval32_to_timeval (tx->time);
> >  
> >    int status = __adjtimex (&tx64);
> >    if (status < 0)  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c
> > b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c index
> > 418efbf546..3935d1cfb5 100644 ---
> > a/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c +++
> > b/sysdeps/unix/sysv/linux/alpha/osf_setitimer.c @@ -30,9 +30,9 @@
> > __setitimer_tv32 (int which, const struct itimerval32 *restrict
> > new_value, { struct itimerval new_value_64;
> >    new_value_64.it_interval
> > -    = valid_timeval_to_timeval64 (new_value->it_interval);
> > +    = valid_timeval32_to_timeval (new_value->it_interval);
> >    new_value_64.it_value
> > -    = valid_timeval_to_timeval64 (new_value->it_value);
> > +    = valid_timeval32_to_timeval (new_value->it_value);
> >  
> >    if (old_value == NULL)
> >      return __setitimer (which, &new_value_64, NULL);  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c
> > b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c index
> > 423c2a8ef2..6c3fad0132 100644 ---
> > a/sysdeps/unix/sysv/linux/alpha/osf_utimes.c +++
> > b/sysdeps/unix/sysv/linux/alpha/osf_utimes.c @@ -28,8 +28,8 @@
> > attribute_compat_text_section __utimes_tv32 (const char *filename,
> > const struct timeval32 times32[2]) {
> >    struct timeval times[2];
> > -  times[0] = valid_timeval_to_timeval64 (times32[0]);
> > -  times[1] = valid_timeval_to_timeval64 (times32[1]);
> > +  times[0] = valid_timeval32_to_timeval (times32[0]);
> > +  times[1] = valid_timeval32_to_timeval (times32[1]);
> >    return __utimes (filename, times);
> >  }
> >    
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h
> > b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h index
> > 6076d2ec05..7169909259 100644 ---
> > a/sysdeps/unix/sysv/linux/alpha/tv32-compat.h +++
> > b/sysdeps/unix/sysv/linux/alpha/tv32-compat.h @@ -70,7 +70,7 @@
> > struct rusage32 overflow, they write { INT32_MAX, TV_USEC_MAX } to
> > the output.  */ 
> >  static inline struct timeval
> > -valid_timeval_to_timeval64 (const struct timeval32 tv)
> > +valid_timeval32_to_timeval (const struct timeval32 tv)
> >  {
> >    return (struct timeval) { tv.tv_sec, tv.tv_usec };
> >  }
> >   
> 
> Ok.




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Attachment: pgpBE0lbrNmVI.pgp
Description: OpenPGP digital signature