This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fifth draft of the Y2038 design document


Hi Paul,

On Wed, 22 Feb 2017 10:45:15 -0800, Paul Eggert <eggert@cs.ucla.edu>
wrote :

> A few comments:
> 
> The spec should clearly state at the start whether it's about the API, 
> or the ABI, or both. Although it appears to be about the API, a lot of 
> ABI concerns are leaking in. If the spec is about both API and ABI, the 
> distinction should be kept clear throught the document.
> 
> The API spec should address only symbols that are part of the API. For 
> example, it should say "pid_t", not "__pid_t", as user code shouldn't 
> know about or use __pid_t. You should go through every 
> leading-underscore symbol in the API spec and decide whether it really 
> needs to be there (as in _TIME_BITS) or should be rewritten (as in __pid_t).

Ok.
 
> In the current spec, if _TIME_BITS is not defined the implementation 
> must behave as if _TIME_BITS==32. This is not future-proof, and will be 
> a bad default in the not-too-distant future. Instead, the spec should 
> say that the default value of _TIME_BITS is implementation-defined. This 
> will let current implementations default _TIME_BITS to 32 now (for 
> backward compatibility to older platforms) while letting future 
> implementations default to 64 (when 32 will make no sense).

Will add this to the doc.

> Some function prototypes have identifiers in the argument names; others 
> don't. Please be consistent and either avoid them everywhere or use them 
> everywhere.

For this document, argument names usually don't matter; I'll remove them
unless the specific context requires them.

> I don't get the point about "a possible implementation" for struct 
> timespec on little-endian vs struct timespec64 on big-endian systems. 
> The implementations are identical.

See Arnd's remark -- there was a typo and a line order mixup (fixed
since, in rev. 117).

> I don't understand the table labeled "Internal changes" with entries 
> like "main(int, char **)" and "xcalloc(int, int)". What do these entries 
> have to do with time? Which "main" and "xcalloc" are we talking about?

This table should be regenerated with file names -- the reason you see
"main(...)" entries is because some file in the GLIBC codebase actually
is a program, with a "main" function, and this "main" function uses
types which are Y2038-incompatible.

> Misspellings: "linux" (should be "Linux"), "adde", "whould", 
> "implementaters", "implementors", "tyime", "clocksettime", 
> "Concersively".

Fixed in rev. 119, thanks.

> Please run a spelling check on it just before the next 
> review.

Will do.

Thanks for your review!

Cordialement,
Albert ARIBAUD
3ADEV


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]