Message ID | 20200126145258.32643-2-lukma@denx.de |
---|---|
State | Superseded |
Headers |
Received: (qmail 39694 invoked by alias); 26 Jan 2020 14:53:28 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 39610 invoked by uid 89); 26 Jan 2020 14:53:28 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 spammy=HX-Languages-Length:1298, time_t X-HELO: mail-out.m-online.net From: Lukasz Majewski <lukma@denx.de> To: Joseph Myers <joseph@codesourcery.com>, Paul Eggert <eggert@cs.ucla.edu>, Adhemerval Zanella <adhemerval.zanella@linaro.org> Cc: Alistair Francis <alistair23@gmail.com>, Alistair Francis <alistair.francis@wdc.com>, GNU C Library <libc-alpha@sourceware.org>, Siddhesh Poyarekar <siddhesh@gotplt.org>, Florian Weimer <fweimer@redhat.com>, Florian Weimer <fw@deneb.enyo.de>, Zack Weinberg <zackw@panix.com>, Carlos O'Donell <carlos@redhat.com>, Andreas Schwab <schwab@suse.de>, Lukasz Majewski <lukma@denx.de> Subject: [PATCH v2 2/5] y2038: Introduce struct __timeval64 - new internal glibc type Date: Sun, 26 Jan 2020 15:52:55 +0100 Message-Id: <20200126145258.32643-2-lukma@denx.de> In-Reply-To: <20200126145258.32643-1-lukma@denx.de> References: <20200126145258.32643-1-lukma@denx.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit |
Commit Message
Lukasz Majewski
Jan. 26, 2020, 2:52 p.m. UTC
This type is a glibc's "internal" type similar to struct timeval but whose tv_sec field is a __time64_t rather than a time_t, which makes it Y2038-proof. This struct is NOT supposed to be passed to the kernel - instead it shall be converted to struct __timespec64 and clock_[sg]ettime syscalls shall be used (which are now Y2038 safe). Build tests: ./src/scripts/build-many-glibcs.py glibcs --- Changes for v2: - Replace __suseconds_t with __suseconds64_t --- include/time.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
Comments
On Mon, Jan 27, 2020 at 12:53 AM Lukasz Majewski <lukma@denx.de> wrote: > > This type is a glibc's "internal" type similar to struct timeval but > whose tv_sec field is a __time64_t rather than a time_t, which makes it > Y2038-proof. This struct is NOT supposed to be passed to the kernel - > instead it shall be converted to struct __timespec64 and clock_[sg]ettime > syscalls shall be used (which are now Y2038 safe). > > Build tests: > ./src/scripts/build-many-glibcs.py glibcs > > --- > Changes for v2: > - Replace __suseconds_t with __suseconds64_t > --- > include/time.h | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/include/time.h b/include/time.h > index 047f431a1a..99492a1577 100644 > --- a/include/time.h > +++ b/include/time.h > @@ -93,6 +93,20 @@ struct __itimerspec64 > }; > #endif > > +#if __TIMESIZE == 64 > +# define __timeval64 timeval > +#else > +/* The glibc Y2038-proof struct __timeval64 structure for a time value. > + This structure is NOT supposed to be passed to the Linux kernel. > + Instead, it shall be converted to struct __timespec64 and time shall > + be [sg]et via clock_[sg]ettime (which are now Y2038 safe). */ > +struct __timeval64 > +{ > + __time64_t tv_sec; /* Seconds */ > + __suseconds64_t tv_usec; /* Microseconds */ > +}; > +#endif Can this be moved to time/bits/types/struct_timeval.h? The reason is that we need it in include/bits/types/struct_rusage.h and including time.h results in exposed symbols (from memory I see glibc test failures when trying to do this). The other problem is that we need __timeval64 in time/sys/time.h for __getitimer64() and __setitimer64() and include/time.h isn't included in there. Alistair > + > #if __TIMESIZE == 64 > # define __ctime64 ctime > #else > -- > 2.20.1 >
On Tue, 28 Jan 2020, Alistair Francis wrote: > Can this be moved to time/bits/types/struct_timeval.h? That's an installed header. Internal types don't belong in installed headers unless unavoidable. Note that the __USE_TIME_BITS64 conditionals we eventually get in installed headers will (for example) change the contents of "struct timeval" depending on __USE_TIME_BITS64, rather than adding a new type with an internal name. > The reason is that we need it in include/bits/types/struct_rusage.h > and including time.h results in exposed symbols (from memory I see > glibc test failures when trying to do this). include/bits/types/struct_rusage.h is a *wrapper* round an installed header. Although such wrappers can contain internal declarations under a !_ISOMAC condition, it's best not to in this case because of the rule that including <bits/types/foo.h> should always get a definition of the type foo and nothing else beyond foo's dependencies, and breaking that rule would be confusing. > The other problem is that we need __timeval64 in time/sys/time.h for > __getitimer64() and __setitimer64() and include/time.h isn't included > in there. But time/sys/time.h is an installed header. So it shouldn't get internal types either. __getitimer64 and __setitimer64 are not public interfaces (nor, as APIs, should they ever be, though eventually __USE_TIME_BITS64 may cause redirection to such names as ABIs). The installed header changes for _TIME_BITS=64 support aren't until all the internal pieces are ready. Until then, to a first approximation, you should only be changing *internal, non-installed* headers and implementation files, not installed headers.
On Mon, Jan 27, 2020 at 4:47 PM Joseph Myers <joseph@codesourcery.com> wrote: > > On Tue, 28 Jan 2020, Alistair Francis wrote: > > > Can this be moved to time/bits/types/struct_timeval.h? > > That's an installed header. Internal types don't belong in installed > headers unless unavoidable. Note that the __USE_TIME_BITS64 conditionals > we eventually get in installed headers will (for example) change the > contents of "struct timeval" depending on __USE_TIME_BITS64, rather than > adding a new type with an internal name. > > > The reason is that we need it in include/bits/types/struct_rusage.h > > and including time.h results in exposed symbols (from memory I see > > glibc test failures when trying to do this). > > include/bits/types/struct_rusage.h is a *wrapper* round an installed > header. Although such wrappers can contain internal declarations under a > !_ISOMAC condition, it's best not to in this case because of the rule that > including <bits/types/foo.h> should always get a definition of the type > foo and nothing else beyond foo's dependencies, and breaking that rule > would be confusing. How should I add a __rusage64 type then? Right now I have modified include/bits/types/struct_rusage.h to include the header twice with different definitions based on macros. > > > The other problem is that we need __timeval64 in time/sys/time.h for > > __getitimer64() and __setitimer64() and include/time.h isn't included > > in there. > > But time/sys/time.h is an installed header. So it shouldn't get internal > types either. __getitimer64 and __setitimer64 are not public interfaces > (nor, as APIs, should they ever be, though eventually __USE_TIME_BITS64 > may cause redirection to such names as ABIs). From memory I was seeing compiler failures, as something doesn't include time.h. I can look into this more though. Alistair > > The installed header changes for _TIME_BITS=64 support aren't until all > the internal pieces are ready. Until then, to a first approximation, you > should only be changing *internal, non-installed* headers and > implementation files, not installed headers. > > -- > Joseph S. Myers > joseph@codesourcery.com
On Tue, 28 Jan 2020, Alistair Francis wrote:
> How should I add a __rusage64 type then? Right now I have modified
Define it in an *internal, non-installed* header, just like __timespec64
is defined in include/time.h (the internal wrapper for the installed
header). For example, inside the "#ifndef _ISOMAC" section of
include/sys/resource.h (if using a wrapper header like that, such internal
definitions must be inside #ifndef _ISOMAC and the include of the public
header outside that conditional).
On Mon, Jan 27, 2020 at 5:42 PM Joseph Myers <joseph@codesourcery.com> wrote: > > On Tue, 28 Jan 2020, Alistair Francis wrote: > > > How should I add a __rusage64 type then? Right now I have modified > > Define it in an *internal, non-installed* header, just like __timespec64 > is defined in include/time.h (the internal wrapper for the installed > header). For example, inside the "#ifndef _ISOMAC" section of > include/sys/resource.h (if using a wrapper header like that, such internal > definitions must be inside #ifndef _ISOMAC and the include of the public > header outside that conditional). Thanks for the help Joseph. I think I have changed it all around so it's in non-installed headers. Alistair > > -- > Joseph S. Myers > joseph@codesourcery.com
diff --git a/include/time.h b/include/time.h index 047f431a1a..99492a1577 100644 --- a/include/time.h +++ b/include/time.h @@ -93,6 +93,20 @@ struct __itimerspec64 }; #endif +#if __TIMESIZE == 64 +# define __timeval64 timeval +#else +/* The glibc Y2038-proof struct __timeval64 structure for a time value. + This structure is NOT supposed to be passed to the Linux kernel. + Instead, it shall be converted to struct __timespec64 and time shall + be [sg]et via clock_[sg]ettime (which are now Y2038 safe). */ +struct __timeval64 +{ + __time64_t tv_sec; /* Seconds */ + __suseconds64_t tv_usec; /* Microseconds */ +}; +#endif + #if __TIMESIZE == 64 # define __ctime64 ctime #else