From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id A4CF11F461 for ; Mon, 2 Sep 2019 18:42:58 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:cc:references:from:subject:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=vDG/Fg/c9JdJII24 6cMViSxs/Bo9yMwwwLUPfVzSS1bAFX8AAvatf3kJA+xaO5K3LZyWpCdw/Agy5Qly 86IKFrf3np68y0iLGNATrU9TBwTst/5bGdWcb4dXWZ2zK+EicR3q/4tDQ/K7KP9z VklalXOpGR7kZsMCfVV5qIfRfTQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:cc:references:from:subject:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=AYom9dDeVQyjO2HeGoV1WE LhaJU=; b=PiB4LTnS0TyGrglePyEZDF579lT6LgieXS4cOp4PsQiSFUlDT2TJ6c uAXFQwK14c0pTTLkfi+cHXgFcjvDpliwg79VjfykB0C7eKF1MRrF0Dy52YYaEvbp D0p7IN4ow30yst4kSSrdzv1R53odc4Ry1NGk5eRsI2W/5jByEpozg= Received: (qmail 26002 invoked by alias); 2 Sep 2019 18:42:56 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 25988 invoked by uid 89); 2 Sep 2019 18:42:55 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-qt1-f195.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=to:cc:references:from:openpgp:autocrypt:subject:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=wBF9TihMPz3D6v0u3C0KfnNZclMzK2ZgRTgc4WpjXXw=; b=S5Uq1NkHrbWecP0lrLBkLV8oB6lGCOMtXOSk3ylYEOiG0QSim4DKMdzr7f6bwU/cpY /sUY9yj66LKz+5pdRs3vSESXC/6sG12XxzCBMxk+UuyOaFBE1V02l4uGpibLw+sDIh0f xkCp9p3RS9UJ2zl53sP9ARXAXPTnLkn4e5e+So1z83hjWulxuDo6sU+PIYQeWRtcD3UX F8vAs793yQlXEwGQDbG8vvDZlPOZSMEl+jpHk4ZZfG2btvB6Bsvp2WjT9jwJCPCe4AXP 9U3wgqFbxlr+5M75X7VPvCSLbGgdeFu0JJV8MgCVKJ9e6J/fjNC0kc4U0lHx+3C+P2XY y9Sw== To: Zack Weinberg , libc-alpha@sourceware.org Cc: Joseph Myers , Florian Weimer , Lukasz Majewski , Alistair Francis , Stepan Golosunov , Arnd Bergmann , Samuel Thibault References: <20190828153236.18229-1-zackw@panix.com> <20190828153236.18229-7-zackw@panix.com> From: Adhemerval Zanella Openpgp: preference=signencrypt Subject: Re: [PATCH v2 06/10] Use clock_gettime to implement ftime; withdraw ftime. Message-ID: Date: Mon, 2 Sep 2019 15:42:46 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190828153236.18229-7-zackw@panix.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 28/08/2019 12:32, Zack Weinberg wrote: > ftime is an obsolete variation on gettimeofday, offering only > millisecond time resolution; it was probably a system call in ooold > versions of BSD Unix. For historic reasons, we had three > implementations of it. These are all consolidated into time/ftime.c, > and then the function is made a compatibility symbol. The public > header file that declared it, its result structure, and nothing else, > is removed. > > For some reason, the implementation of ftime in terms of gettimeofday > was rounding rather than truncating microseconds to milliseconds. In > all the other places where we use a higher-resolution time function to > implement a lower-resolution one, we truncate. ftime is changed to > match, just for tidiness' sake. Sound reasonable. > > Like gettimeofday, ftime tries to report the time zone, and using that > information is always a bug. This patch dummies out the reported > timezone information; the timezone and dstflag fields of the > returned "struct timeb" will always be zero. Ok. > > * time/tst-ftime.c > * time/sys/timeb.h > * sysdeps/unix/bsd/ftime.c > * sysdeps/unix/sysv/linux/ftime.c: Delete file. > * time/tst-ftime_l.c: Rename to tst-strftime_l.c. > > * time/ftime.c (ftime): Make a compatibility-only symbol. > Replace implementation with the code formerly in > sysdeps/unix/bsd/ftime.c, then change that code to use > __clock_gettime instead of __gettimeofday and to truncate > rather than rounding. Always set the timezone and dstflag > fields of the timebuf argument to zero. > > * time/Makefile (headers): Remove sys/timeb.h. > (tests): Remove tst-ftime, change tst-ftime_l to > tst-strftime_l. > > * conform/Makefile: XFAIL all tests related to sys/timeb.h. > --- > conform/Makefile | 9 ++++ > sysdeps/unix/bsd/ftime.c | 40 ---------------- > sysdeps/unix/sysv/linux/ftime.c | 3 -- > time/Makefile | 8 ++-- > time/ftime.c | 40 +++++++++------- > time/sys/timeb.h | 43 ------------------ > time/tst-ftime.c | 58 ------------------------ > time/{tst-ftime_l.c => tst-strftime_l.c} | 0 > 8 files changed, 36 insertions(+), 165 deletions(-) > delete mode 100644 sysdeps/unix/bsd/ftime.c > delete mode 100644 sysdeps/unix/sysv/linux/ftime.c > delete mode 100644 time/sys/timeb.h > delete mode 100644 time/tst-ftime.c > rename time/{tst-ftime_l.c => tst-strftime_l.c} (100%) > > diff --git a/conform/Makefile b/conform/Makefile > index 59d569c4c5..8671c695e8 100644 > --- a/conform/Makefile > +++ b/conform/Makefile > @@ -241,3 +241,12 @@ test-xfail-XPG42/ndbm.h/linknamespace = yes > test-xfail-UNIX98/ndbm.h/linknamespace = yes > test-xfail-XOPEN2K/ndbm.h/linknamespace = yes > test-xfail-XOPEN2K8/ndbm.h/linknamespace = yes > + > +# Header no longer provided by glibc (obsoleted in newer POSIX > +# standards). > +test-xfail-UNIX98/sys/timeb.h/conform = yes > +test-xfail-UNIX98/sys/timeb.h/linknamespace = yes > +test-xfail-XOPEN2K/sys/timeb.h/conform = yes > +test-xfail-XOPEN2K/sys/timeb.h/linknamespace = yes > +test-xfail-XPG42/sys/timeb.h/conform = yes > +test-xfail-XPG42/sys/timeb.h/linknamespace = yes I think we will need to keep this header, at least one release, and mark is as deprecated (maybe with a pragma warning) because some projects include without configure checks (libsanitizer for instance). > diff --git a/sysdeps/unix/bsd/ftime.c b/sysdeps/unix/bsd/ftime.c > deleted file mode 100644 > index 3a1c6e9b01..0000000000 > --- a/sysdeps/unix/bsd/ftime.c > +++ /dev/null > @@ -1,40 +0,0 @@ > -/* Copyright (C) 1994-2019 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - . */ > - > -#include > -#include > - > -int > -ftime (struct timeb *timebuf) > -{ > - struct timeval tv; > - struct timezone tz; > - > - if (__gettimeofday (&tv, &tz) < 0) > - return -1; > - > - timebuf->time = tv.tv_sec; > - timebuf->millitm = (tv.tv_usec + 500) / 1000; > - if (timebuf->millitm == 1000) > - { > - ++timebuf->time; > - timebuf->millitm = 0; > - } > - timebuf->timezone = tz.tz_minuteswest; > - timebuf->dstflag = tz.tz_dsttime; > - return 0; > -} Ok. > diff --git a/sysdeps/unix/sysv/linux/ftime.c b/sysdeps/unix/sysv/linux/ftime.c > deleted file mode 100644 > index 5a5949f608..0000000000 > --- a/sysdeps/unix/sysv/linux/ftime.c > +++ /dev/null > @@ -1,3 +0,0 @@ > -/* Linux defines the ftime system call but doesn't actually implement > - it. Use the BSD implementation. */ > -#include > diff --git a/time/Makefile b/time/Makefile > index 791db1c8e3..93c7cc28d4 100644 > --- a/time/Makefile > +++ b/time/Makefile > @@ -22,7 +22,7 @@ subdir := time > > include ../Makeconfig > > -headers := time.h sys/time.h sys/timeb.h bits/time.h \ > +headers := time.h sys/time.h bits/time.h \ > bits/types/clockid_t.h bits/types/clock_t.h \ > bits/types/struct_itimerspec.h \ > bits/types/struct_timespec.h bits/types/struct_timeval.h \ As before. > @@ -43,9 +43,9 @@ routines := offtime asctime clock ctime ctime_r difftime \ > aux := era alt_digit lc-time-cleanup > > tests := test_time clocktest tst-posixtz tst-strptime tst_wcsftime \ > - tst-getdate tst-mktime tst-mktime2 tst-ftime_l tst-strftime \ > + tst-getdate tst-mktime tst-mktime2 tst-strftime tst-strftime_l \ > tst-mktime3 tst-strptime2 bug-asctime bug-asctime_r bug-mktime1 \ > - tst-strptime3 bug-getdate1 tst-strptime-whitespace tst-ftime \ > + tst-strptime3 bug-getdate1 tst-strptime-whitespace \ > tst-tzname tst-y2039 bug-mktime4 tst-strftime2 tst-strftime3 \ > tst-clock tst-clock2 tst-clock_nanosleep tst-cpuclock1 > > @@ -59,7 +59,7 @@ LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP fr_FR.UTF-8 \ > nan_TW.UTF-8 lzh_TW.UTF-8 > include ../gen-locales.mk > > -$(objpfx)tst-ftime_l.out: $(gen-locales) > +$(objpfx)tst-strftime_l.out: $(gen-locales) > $(objpfx)tst-strptime.out: $(gen-locales) > endif > > diff --git a/time/ftime.c b/time/ftime.c > index 6c2a256048..0db6b70adf 100644 > --- a/time/ftime.c > +++ b/time/ftime.c > @@ -15,27 +15,33 @@ > License along with the GNU C Library; if not, see > . */ > > -#include > -#include > -#include > +#include > > -int > -ftime (struct timeb *timebuf) > -{ > - int save = errno; > - struct tm tp; > +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_31) > > - __set_errno (0); > - if (time (&timebuf->time) == (time_t) -1 && errno != 0) > - return -1; > - timebuf->millitm = 0; > +#include > > - if (__localtime_r (&timebuf->time, &tp) == NULL) > - return -1; > +struct timeb > +{ > + time_t time; /* Seconds since epoch, as from `time'. */ > + unsigned short int millitm; /* Additional milliseconds. */ > + short int timezone; /* Minutes west of GMT. */ > + short int dstflag; /* Nonzero if Daylight Savings Time used. */ > +}; > > - timebuf->timezone = tp.tm_gmtoff / 60; > - timebuf->dstflag = tp.tm_isdst; > +int > +attribute_compat_text_section > +__ftime (struct timeb *timebuf) > +{ > + struct timespec ts; > + __clock_gettime (CLOCK_REALTIME, &ts); > > - __set_errno (save); > + timebuf->time = ts.tv_sec; > + timebuf->millitm = ts.tv_nsec / 1000000; > + timebuf->timezone = 0; > + timebuf->dstflag = 0; > return 0; > } > +compat_symbol (libc, __ftime, ftime, GLIBC_2_0); > + > +#endif Ok. > diff --git a/time/sys/timeb.h b/time/sys/timeb.h > deleted file mode 100644 > index 6333e8074d..0000000000 > --- a/time/sys/timeb.h > +++ /dev/null > @@ -1,43 +0,0 @@ > -/* Copyright (C) 1994-2019 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - . */ > - > -#ifndef _SYS_TIMEB_H > -#define _SYS_TIMEB_H 1 > - > -#include > - > -#include > - > -__BEGIN_DECLS > - > -/* Structure returned by the `ftime' function. */ > - > -struct timeb > - { > - time_t time; /* Seconds since epoch, as from `time'. */ > - unsigned short int millitm; /* Additional milliseconds. */ > - short int timezone; /* Minutes west of GMT. */ > - short int dstflag; /* Nonzero if Daylight Savings Time used. */ > - }; > - > -/* Fill in TIMEBUF with information about the current time. */ > - > -extern int ftime (struct timeb *__timebuf); > - > -__END_DECLS > - > -#endif /* sys/timeb.h */ > diff --git a/time/tst-ftime.c b/time/tst-ftime.c > deleted file mode 100644 > index 65b753dec8..0000000000 > --- a/time/tst-ftime.c > +++ /dev/null > @@ -1,58 +0,0 @@ > -/* Verify that ftime is sane. > - Copyright (C) 2014-2019 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - . */ > - > -#include > -#include > - > -static int > -do_test (void) > -{ > - struct timeb prev, curr = {.time = 0, .millitm = 0}; > - int sec = 0; > - > - while (sec != 3) > - { > - prev = curr; > - > - if (ftime (&curr)) > - { > - printf ("ftime returned an error\n"); > - return 1; > - } > - > - if (curr.time < prev.time) > - { > - printf ("ftime's time flowed backwards\n"); > - return 1; > - } > - > - if (curr.time == prev.time > - && curr.millitm < prev.millitm) > - { > - printf ("ftime's millitm flowed backwards\n"); > - return 1; > - } > - > - if (curr.time > prev.time) > - sec ++; > - } > - return 0; > -} > - > -#define TEST_FUNCTION do_test () > -#include "../test-skeleton.c" We can make it a internal test using TEST_COMPAT instead. > diff --git a/time/tst-ftime_l.c b/time/tst-strftime_l.c > similarity index 100% > rename from time/tst-ftime_l.c > rename to time/tst-strftime_l.c >