Hi Joseph,

> On Sat, 5 Dec 2020, Lukasz Majewski wrote:
> 
> > This change introduces new compile time, supported by glibc flag -
> > namely _TIME_BITS, which then is responsible for exporting
> > __USE_TIME_BITS64 only  
> 
> As a major new user-visible feature, this needs to be mentioned in
> NEWS.

Ok.

> 
> > +/* We need to know the word size in order to check the time size.
> > */ +#include <bits/wordsize.h>
> > +
> > +#if defined _TIME_BITS
> > +# if _TIME_BITS == 64
> > +#  if ! defined (_FILE_OFFSET_BITS) || _FILE_OFFSET_BITS != 64
> > +#   error _TIME_BITS==64 is allowed only when _FILE_OFFSET_BITS==64
> > +#  elif __WORDSIZE == 32
> > +#   define __USE_TIME_BITS64	1
> > +#  endif  
> 
> __WORDSIZE isn't the right thing to check.  On ARC or RV32 or x32, 
> _TIME_BITS == 64 should be a no-op, because time_t has always been
> 64-bit for those glibc ports.  That is, you should be checking
> __TIMESIZE, not __WORDSIZE.

Ok. Thanks for pointing this out

> 
> > +# elif _TIME_BITS == 32
> > +#  if __WORDSIZE > 32
> > +#   error _TIME_BITS=32 is not compatible with __WORDSIZE > 32  
> 
> Likewise, __TIMESIZE should be checked here, not __WORDSIZE.
> 

Ok.

> > +@defvr Macro _TIME_BITS
> > +This macro determines the bit size of @code{time_t} (and therefore
> > the +bit size of all @code{time_t} derived types and the prototypes
> > of all +related functions). If @code{_TIME_BITS} is undefined, the
> > bit size of +time_t equals the bit size of the architecture.  
> 
> Apart from "bit size of the architecture" not being well-defined,
> that's not right for ARC or RV32, 32-bit architectures with 64-bit
> time_t.
> 
> Note there should be two spaces after "." in Texinfo manuals.
> 
> > +If @code{_TIME_BITS} is undefined, or if @code{_TIME_BITS} is
> > defined +to the value @code{32} and @code{__WORDSIZE} is defined to
> > the value +@code{32}, or or if @code{_TIME_BITS} is defined to the
> > value @code{64} +and @code{__WORDSIZE} is defined to the value
> > @code{64}, nothing changes.  
> 
> __WORDSIZE (and __TIMESIZE after the fix above) is not a user-visible 
> interface, it's a glibc header implementation detail.  The manual
> (other than maint.texi) should not mention either such internal
> macro; you need to talk about a concept of word size / default time_t
> size that's explained purely in user-level interfaces, not glibc
> internal macros.
> 
> > +If @code{_TIME_BITS} is defined to the value @code{64} and if
> > +@code{__WORDSIZE} is defined to the value @code{32}, then the
> > @w{64 bit} +time API and implementation are used even though the
> > architecture word +size is @code{32}. Also, if the kernel provides
> > @w{64 bit} time support, +it is used; otherwise, the @w{32 bit}
> > kernel time support is used (with +no provision to address kernel
> > Y2038 shortcomings).  
> 
> Use "32-bit" and "64-bit" (with hyphen) as adjectives.
> 

Thanks for pointing this out - I will rewrite the manual entry.


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