Re: [PATCH v9] y2038: Introduce the __ASSUME_TIME64_SYSCALLS define

[Lukasz Majewski <lukma at denx dot de>]

Hi Alistair,

> On Tue, Aug 27, 2019 at 12:35 PM Joseph Myers
> <joseph@codesourcery.com> wrote:
> >
> > On Tue, 27 Aug 2019, Zack Weinberg wrote:
> >  
> > > I think that gets everything important.  Neither the commit
> > > message nor the comment needs to list all the new system calls.  
> >
> > The syscalls need to be listed to make clear exactly what the
> > interfaces covered by the macro are.  
> 
> I'm a little confused, should I change anything or leave the patch as
> is?

Although, I do have the little experience in the glibc development, I
would like to propose the patch description (commit message and
in-header comment) as in v8:

https://patchwork.ozlabs.org/patch/1117100/

The commit message is very short as the all info is in the comment.

Moreover, I would like to point out that the v8 had issues with wording
and IMHO in general reflected the idea behind __ASSUME_TIME64_SYSCALLS
flag.

Maybe we could refine v8 and v9 (reworded by Alistair after Joseph's
comments) ?

Zack, would you consider re-reading v8 and v9 and refining them ?

> 
> >
> > * If further related syscalls are added in future, e.g. timespec64
> > versions of syscalls that currently use struct rusage, they are
> > *not* covered by this macro.
> >
> > * If the semantics of the unsuffixed syscall on 64-bit
> > architectures do not exactly match those of the suffixed syscall on
> > 32-bit architectures, so that a #define of the suffixed name to the
> > unsuffixed name doesn't suffice to use the existing syscall on
> > 64-bit architectures, it is *not* covered by this macro, at least
> > not without an explanation of the semantic differences in the
> > comment.  (See the semtimedop discussion.)
> >
> > * If the unsuffixed syscall is not in fact available on all 64-bit
> > architectures supported by glibc in the minimum supported kernel
> > version, it is *not* covered by this macro.  (Although treating it
> > as covered would only cause issues if there is actually any
> > fallback code for the case where the unsuffixed syscall isn't
> > present either.)
> >
> > The fact that the syscalls described by the macro may be suffixed or
> > unsuffixed also needs to be in the comment, not just the commit
> > message.  
> 
> Is this enough to describe that (already in the commit)?
> 
>    On systems with __WORDSIZE == 64 the __NR_clock_settime syscall is
> used to achieve this goal. Systems with __WORDSIZE == 32 use the
>    __NR_clock_settime64 syscall available from Linux version 5.1.
> 
> Alistair
> 
> >
> > --
> > Joseph S. Myers
> > joseph@codesourcery.com  



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: pgpillCWZsyhv.pgp
Description: OpenPGP digital signature