[2/3] Move ftime to a compatibility symbol

Message ID 20201015130632.90961-2-adhemerval.zanella@linaro.org
State Committed
Commit 14633d3e568eb9770a7e5046eff257113e0453fb
Headers
Series [1/3] linux: Fix time64 support for futimesat |

Commit Message

Adhemerval Zanella Oct. 15, 2020, 1:06 p.m. UTC
  It was made deprecated on 2.31, so it moves to compat symbol after
two releases.  It was also removed from exported symbol for riscv32
(since ABI will be supported on for 2.33).

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 include/sys/timeb.h                           |  1 -
 .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
 time/Makefile                                 |  5 +-
 time/ftime.c                                  | 19 +++++-
 time/sys/timeb.h                              | 44 --------------
 time/tst-ftime.c                              | 59 ++++++++++---------
 6 files changed, 50 insertions(+), 79 deletions(-)
 delete mode 100644 include/sys/timeb.h
 delete mode 100644 time/sys/timeb.h
  

Comments

Lukasz Majewski Oct. 16, 2020, 9:05 a.m. UTC | #1
Hi Adhemerval,

> It was made deprecated on 2.31, so it moves to compat symbol after
> two releases.  It was also removed from exported symbol for riscv32
> (since ABI will be supported on for 2.33).
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  include/sys/timeb.h                           |  1 -
>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
>  time/Makefile                                 |  5 +-
>  time/ftime.c                                  | 19 +++++-
>  time/sys/timeb.h                              | 44 --------------
>  time/tst-ftime.c                              | 59
> ++++++++++--------- 6 files changed, 50 insertions(+), 79 deletions(-)
>  delete mode 100644 include/sys/timeb.h
>  delete mode 100644 time/sys/timeb.h
> 
> diff --git a/include/sys/timeb.h b/include/sys/timeb.h
> deleted file mode 100644
> index 9f4509c35e..0000000000
> --- a/include/sys/timeb.h
> +++ /dev/null
> @@ -1 +0,0 @@
> -#include <time/sys/timeb.h>
> diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
> b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist index
> 36aa34a5e7..e977715088 100644 ---
> a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist +++
> b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist @@ -795,7 +795,6 @@
> GLIBC_2.33 fsync F GLIBC_2.33 ftell F
>  GLIBC_2.33 ftello F
>  GLIBC_2.33 ftello64 F
> -GLIBC_2.33 ftime F
>  GLIBC_2.33 ftok F
>  GLIBC_2.33 ftruncate F
>  GLIBC_2.33 ftruncate64 F
> diff --git a/time/Makefile b/time/Makefile
> index a4fb13d6a3..ab8fb3303b 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	\ @@ -45,9 +45,10 @@ 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-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
> +tests-internal := tst-ftime
>  
>  include ../Rules
>  
> diff --git a/time/ftime.c b/time/ftime.c
> index 93f485bbf7..be3295ef76 100644
> --- a/time/ftime.c
> +++ b/time/ftime.c
> @@ -16,11 +16,23 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#include <sys/timeb.h>
> +#include <shlib-compat.h>
> +
> +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_33)
> +
>  #include <time.h>
>  
> +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.  */
> +  };
> +
>  int
> -ftime (struct timeb *timebuf)
> +attribute_compat_text_section
> +__ftime (struct timeb *timebuf)
>  {
>    struct timespec ts;
>    __clock_gettime (CLOCK_REALTIME, &ts);
> @@ -31,3 +43,6 @@ ftime (struct timeb *timebuf)
>    timebuf->dstflag = 0;
>    return 0;
>  }
> +
> +compat_symbol (libc, __ftime, ftime, GLIBC_2_0);
> +#endif
> diff --git a/time/sys/timeb.h b/time/sys/timeb.h
> deleted file mode 100644
> index 641c333450..0000000000
> --- a/time/sys/timeb.h
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -/* Copyright (C) 1994-2020 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
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#ifndef _SYS_TIMEB_H
> -#define _SYS_TIMEB_H	1
> -
> -#include <features.h>
> -
> -#include <bits/types/time_t.h>
> -
> -__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)
> -  __nonnull ((1)) __attribute_deprecated__;
> -
> -__END_DECLS
> -
> -#endif	/* sys/timeb.h */

One less exported header to maintain. Good :-)

> diff --git a/time/tst-ftime.c b/time/tst-ftime.c
> index 08916c0c65..6978feb0f1 100644
> --- a/time/tst-ftime.c
> +++ b/time/tst-ftime.c
> @@ -16,9 +16,23 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#include <sys/timeb.h>
> -#include <stdio.h>
> -#include <libc-diag.h>
> +
> +#include <shlib-compat.h>
> +#if TEST_COMPAT (libc, GLIBC_2_0, GLIBC_2_33)
> +#include <time.h>
> +#include <support/check.h>
> +
> +compat_symbol_reference (libc, ftime, ftime, GLIBC_2_0);
> +
> +struct timeb
> +  {
> +    time_t time;
> +    unsigned short int millitm;
> +    short int timezone;
> +    short int dstflag;
> +  };
> +
> +extern int ftime (struct timeb *__timebuf);
>  
>  static int
>  do_test (void)
> @@ -30,36 +44,23 @@ do_test (void)
>      {
>        prev = curr;
>  
> -      /* ftime was deprecated on 2.31.  */
> -      DIAG_PUSH_NEEDS_COMMENT;
> -      DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Wdeprecated-declarations");
> -
> -      if (ftime (&curr))
> -        {
> -          printf ("ftime returned an error\n");
> -          return 1;
> -        }
> -
> -      DIAG_POP_NEEDS_COMMENT;
> -
> -      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;
> -        }
> +      /* ftime was deprecated on 2.31 and removed on 2.33.  */
> +      TEST_COMPARE (ftime (&curr), 0);
> +      TEST_VERIFY_EXIT (curr.time >= prev.time);
> +      if (curr.time == prev.time)
> +	TEST_VERIFY_EXIT (curr.millitm >= prev.millitm);
>  
>        if (curr.time > prev.time)
>          sec ++;
>      }
>    return 0;
>  }
> +#else
> +static int
> +do_test (void)
> +{
> +  return EXIT_UNSUPPORTED;
> +}
> +#endif
>  
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>

Reviewed-by: Lukasz Majewski <lukma@denx.de>


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
  
Szabolcs Nagy Oct. 19, 2020, 7:55 a.m. UTC | #2
The 10/15/2020 10:06, Adhemerval Zanella via Libc-alpha wrote:
> It was made deprecated on 2.31, so it moves to compat symbol after
> two releases.  It was also removed from exported symbol for riscv32
> (since ABI will be supported on for 2.33).
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  include/sys/timeb.h                           |  1 -
>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
>  time/Makefile                                 |  5 +-
>  time/ftime.c                                  | 19 +++++-
>  time/sys/timeb.h                              | 44 --------------
>  time/tst-ftime.c                              | 59 ++++++++++---------
>  6 files changed, 50 insertions(+), 79 deletions(-)
>  delete mode 100644 include/sys/timeb.h
>  delete mode 100644 time/sys/timeb.h

removing sys/timeb.h and ftime breaks several
spec2006 and spec2017 benchmarks.

which means this header is used in practice,
sometimes without configure checks.

is there a reason it cannot be supported?
  
Andreas Schwab Oct. 19, 2020, 8:17 a.m. UTC | #3
On Okt 19 2020, Szabolcs Nagy via Libc-alpha wrote:

> is there a reason it cannot be supported?

The interface has been removed from POSIX 12+ years ago.

Andreas.
  
Florian Weimer Oct. 19, 2020, 8:30 a.m. UTC | #4
* Szabolcs Nagy via Libc-alpha:

> The 10/15/2020 10:06, Adhemerval Zanella via Libc-alpha wrote:
>> It was made deprecated on 2.31, so it moves to compat symbol after
>> two releases.  It was also removed from exported symbol for riscv32
>> (since ABI will be supported on for 2.33).
>> 
>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>> ---
>>  include/sys/timeb.h                           |  1 -
>>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
>>  time/Makefile                                 |  5 +-
>>  time/ftime.c                                  | 19 +++++-
>>  time/sys/timeb.h                              | 44 --------------
>>  time/tst-ftime.c                              | 59 ++++++++++---------
>>  6 files changed, 50 insertions(+), 79 deletions(-)
>>  delete mode 100644 include/sys/timeb.h
>>  delete mode 100644 time/sys/timeb.h
>
> removing sys/timeb.h and ftime breaks several
> spec2006 and spec2017 benchmarks.
>
> which means this header is used in practice,
> sometimes without configure checks.
>
> is there a reason it cannot be supported?

I think we should minimize the number of time-related symbols, in case
we ever want to provide a dual ABI (with multiple time_t sizes).

SPEC isn't going to change, so I don't think it makes sense to wait for
it getting fixed.

Thanks,
Florian
  
Tamar Christina Oct. 19, 2020, 9:18 a.m. UTC | #5
> -----Original Message-----
> From: Libc-alpha <libc-alpha-bounces@sourceware.org> On Behalf Of Florian
> Weimer via Libc-alpha
> Sent: Monday, October 19, 2020 9:31 AM
> To: Szabolcs Nagy via Libc-alpha <libc-alpha@sourceware.org>
> Subject: Re: [PATCH 2/3] Move ftime to a compatibility symbol
> 
> * Szabolcs Nagy via Libc-alpha:
> 
> > The 10/15/2020 10:06, Adhemerval Zanella via Libc-alpha wrote:
> >> It was made deprecated on 2.31, so it moves to compat symbol after
> >> two releases.  It was also removed from exported symbol for riscv32
> >> (since ABI will be supported on for 2.33).
> >>
> >> Checked on x86_64-linux-gnu and i686-linux-gnu.
> >> ---
> >>  include/sys/timeb.h                           |  1 -
> >>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
> >>  time/Makefile                                 |  5 +-
> >>  time/ftime.c                                  | 19 +++++-
> >>  time/sys/timeb.h                              | 44 --------------
> >>  time/tst-ftime.c                              | 59 ++++++++++---------
> >>  6 files changed, 50 insertions(+), 79 deletions(-)  delete mode
> >> 100644 include/sys/timeb.h  delete mode 100644 time/sys/timeb.h
> >
> > removing sys/timeb.h and ftime breaks several
> > spec2006 and spec2017 benchmarks.
> >
> > which means this header is used in practice, sometimes without
> > configure checks.
> >
> > is there a reason it cannot be supported?
> 
> I think we should minimize the number of time-related symbols, in case we
> ever want to provide a dual ABI (with multiple time_t sizes).
> 
> SPEC isn't going to change, so I don't think it makes sense to wait for it getting
> fixed.

That's a pretty hard line to draw.. Yes released version of spec are unlikely to change but
doesn't mean future versions won't. Has anyone ever pointed this out to the committee?

Regardless the issue now exists that we can't use glibc for benchmark anymore and I don't
really see a workaround.  Yes we can use older glibc versions but then what's the point,
we'll never see any of the improvements we make anymore.

Is there a viable workaround for this? Or do I have to now carry my own extra headers.

Regards,
Tamar

> 
> Thanks,
> Florian
> --
> Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
> Commercial register: Amtsgericht Muenchen, HRB 153243, Managing
> Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill
  
Florian Weimer Oct. 19, 2020, 9:51 a.m. UTC | #6
* Tamar Christina:

>> -----Original Message-----
>> From: Libc-alpha <libc-alpha-bounces@sourceware.org> On Behalf Of Florian
>> Weimer via Libc-alpha
>> Sent: Monday, October 19, 2020 9:31 AM
>> To: Szabolcs Nagy via Libc-alpha <libc-alpha@sourceware.org>
>> Subject: Re: [PATCH 2/3] Move ftime to a compatibility symbol
>> 
>> * Szabolcs Nagy via Libc-alpha:
>> 
>> > The 10/15/2020 10:06, Adhemerval Zanella via Libc-alpha wrote:
>> >> It was made deprecated on 2.31, so it moves to compat symbol after
>> >> two releases.  It was also removed from exported symbol for riscv32
>> >> (since ABI will be supported on for 2.33).
>> >>
>> >> Checked on x86_64-linux-gnu and i686-linux-gnu.
>> >> ---
>> >>  include/sys/timeb.h                           |  1 -
>> >>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
>> >>  time/Makefile                                 |  5 +-
>> >>  time/ftime.c                                  | 19 +++++-
>> >>  time/sys/timeb.h                              | 44 --------------
>> >>  time/tst-ftime.c                              | 59 ++++++++++---------
>> >>  6 files changed, 50 insertions(+), 79 deletions(-)  delete mode
>> >> 100644 include/sys/timeb.h  delete mode 100644 time/sys/timeb.h
>> >
>> > removing sys/timeb.h and ftime breaks several
>> > spec2006 and spec2017 benchmarks.
>> >
>> > which means this header is used in practice, sometimes without
>> > configure checks.
>> >
>> > is there a reason it cannot be supported?
>> 
>> I think we should minimize the number of time-related symbols, in case we
>> ever want to provide a dual ABI (with multiple time_t sizes).
>> 
>> SPEC isn't going to change, so I don't think it makes sense to wait
>> for it getting fixed.
>
> That's a pretty hard line to draw.. Yes released version of spec are
> unlikely to change but doesn't mean future versions won't. Has anyone
> ever pointed this out to the committee?

Has SPEC ever fixed any standards violations?  I don't have access to
the SPEC web site, so I don't know if they have released patches to fix
conformance bugs in benchmarks.  (I recall several discussions about
aliasing violations in older benchmarks on the GCC lists.)

> Regardless the issue now exists that we can't use glibc for benchmark
> anymore and I don't really see a workaround.  Yes we can use older
> glibc versions but then what's the point, we'll never see any of the
> improvements we make anymore.

How does SPEC run on platforms that do not have ftime?  Even on
UNIX-like systems, using ftime presently requires linking with some
compatibility library.  Is there really no knob to use gettimeofday or
clock_gettime instead of ftime?

Thanks,
Florian
  
Tamar Christina Oct. 19, 2020, 10:20 a.m. UTC | #7
> -----Original Message-----
> From: Florian Weimer <fweimer@redhat.com>
> Sent: Monday, October 19, 2020 10:51 AM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: libc-alpha@sourceware.org
> Subject: Re: [PATCH 2/3] Move ftime to a compatibility symbol
> 
> * Tamar Christina:
> 
> >> -----Original Message-----
> >> From: Libc-alpha <libc-alpha-bounces@sourceware.org> On Behalf Of
> >> Florian Weimer via Libc-alpha
> >> Sent: Monday, October 19, 2020 9:31 AM
> >> To: Szabolcs Nagy via Libc-alpha <libc-alpha@sourceware.org>
> >> Subject: Re: [PATCH 2/3] Move ftime to a compatibility symbol
> >>
> >> * Szabolcs Nagy via Libc-alpha:
> >>
> >> > The 10/15/2020 10:06, Adhemerval Zanella via Libc-alpha wrote:
> >> >> It was made deprecated on 2.31, so it moves to compat symbol after
> >> >> two releases.  It was also removed from exported symbol for
> >> >> riscv32 (since ABI will be supported on for 2.33).
> >> >>
> >> >> Checked on x86_64-linux-gnu and i686-linux-gnu.
> >> >> ---
> >> >>  include/sys/timeb.h                           |  1 -
> >> >>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 -
> >> >>  time/Makefile                                 |  5 +-
> >> >>  time/ftime.c                                  | 19 +++++-
> >> >>  time/sys/timeb.h                              | 44 --------------
> >> >>  time/tst-ftime.c                              | 59 ++++++++++---------
> >> >>  6 files changed, 50 insertions(+), 79 deletions(-)  delete mode
> >> >> 100644 include/sys/timeb.h  delete mode 100644 time/sys/timeb.h
> >> >
> >> > removing sys/timeb.h and ftime breaks several
> >> > spec2006 and spec2017 benchmarks.
> >> >
> >> > which means this header is used in practice, sometimes without
> >> > configure checks.
> >> >
> >> > is there a reason it cannot be supported?
> >>
> >> I think we should minimize the number of time-related symbols, in
> >> case we ever want to provide a dual ABI (with multiple time_t sizes).
> >>
> >> SPEC isn't going to change, so I don't think it makes sense to wait
> >> for it getting fixed.
> >
> > That's a pretty hard line to draw.. Yes released version of spec are
> > unlikely to change but doesn't mean future versions won't. Has anyone
> > ever pointed this out to the committee?
> 
> Has SPEC ever fixed any standards violations?  I don't have access to the SPEC
> web site, so I don't know if they have released patches to fix conformance
> bugs in benchmarks.  (I recall several discussions about aliasing violations in
> older benchmarks on the GCC lists.)

They do, and have in the past, but while I can reasonably get one for SPEC2017,
SPEC2006 is no longer maintained as far as I am aware.

> 
> > Regardless the issue now exists that we can't use glibc for benchmark
> > anymore and I don't really see a workaround.  Yes we can use older
> > glibc versions but then what's the point, we'll never see any of the
> > improvements we make anymore.
> 
> How does SPEC run on platforms that do not have ftime?  Even on UNIX-like
> systems, using ftime presently requires linking with some compatibility
> library.  Is there really no knob to use gettimeofday or clock_gettime instead
> of ftime?

If the header was still there but empty I could do something at link time, but
since it doesn't exist at all I have to provide my own local headers.  Also if you
have a list of interfaces glibc plans to remove we can also feed this on to them
to avoid in the next SPEC release.  https://www.spec.org/cpuv8/

Most of the SPEC2017 benchmarks do the right thing with a include guard,
but xalancbmk doesn't, which was presumable taken directly from the
2006 version which also fails (along with h264 in 2006).

I will contact the committee but it seems highly unlikely that they would
Issue a fix for 2006.

Regards,
Tamar

> 
> Thanks,
> Florian
> --
> Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
> Commercial register: Amtsgericht Muenchen, HRB 153243, Managing
> Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill
  
Florian Weimer Oct. 19, 2020, 10:46 a.m. UTC | #8
* Tamar Christina:

>> Has SPEC ever fixed any standards violations?  I don't have access to the SPEC
>> web site, so I don't know if they have released patches to fix conformance
>> bugs in benchmarks.  (I recall several discussions about aliasing violations in
>> older benchmarks on the GCC lists.)
>
> They do, and have in the past, but while I can reasonably get one for
> SPEC2017, SPEC2006 is no longer maintained as far as I am aware.

Here they refused it for SPEC2017:

| q1. Will SPEC fix spec_qsort.c?
| 
| a1. No. There are two reasons:
| 
|     That's the way it is in real life
|     The module spec_qsort.c is based directly on the BSD version, as updated Thu Mar 5 17:17:11 2015 UTC in revision 279666: https://svnweb.freebsd.org/base/head/lib/libc/stdlib/qsort.c?revision=279666.
|     No moving targets
|     SPEC CPU®2017 has already been released. Once source code has been released for a benchmark, SPEC strongly prefers not to change the source code unless there are compelling portability reasons.
|
| Note, therefore, that the patch attached to GCC bug 83201 is not approved by SPEC and would not be allowed in a reportable run.

<https://www.spec.org/cpu2017/Docs/benchmarks/505.mcf_r.html>

> If the header was still there but empty I could do something at link
> time, but since it doesn't exist at all I have to provide my own local
> headers.  Also if you have a list of interfaces glibc plans to remove
> we can also feed this on to them to avoid in the next SPEC release.
> https://www.spec.org/cpuv8/

There is no such list.  There isn't even consensus among glibc
developers that it's necessary to provide a deprecation notice before
removal.  Building with an appropriate version of _POSIX_SOURCE can help.

A good indicator is whether a function has been removed by POSIX.
Sometimes the manual pages also mark a function as obsolete.  Usually,
that's sufficient reason to stay away from it.  For ftime, this happened
some time before 2004.

Thanks,
Florian
  
Florian Weimer Oct. 19, 2020, 10:49 a.m. UTC | #9
* Adhemerval Zanella via Libc-alpha:

> It was made deprecated on 2.31, so it moves to compat symbol after
> two releases.  It was also removed from exported symbol for riscv32
> (since ABI will be supported on for 2.33).

Why doesn't this break the conformance tests for
-D_POSIX_C_SOURCE=200112L?

Have we ever turned anything into a compatibility symbol that is
declared in a standard header?

Thanks,
Florian
  
Andreas Schwab Oct. 19, 2020, 11:56 a.m. UTC | #10
On Okt 19 2020, Florian Weimer via Libc-alpha wrote:

> * Adhemerval Zanella via Libc-alpha:
>
>> It was made deprecated on 2.31, so it moves to compat symbol after
>> two releases.  It was also removed from exported symbol for riscv32
>> (since ABI will be supported on for 2.33).
>
> Why doesn't this break the conformance tests for
> -D_POSIX_C_SOURCE=200112L?
>
> Have we ever turned anything into a compatibility symbol that is
> declared in a standard header?

I think it should be handled the same way as gets.

Andreas.
  
Florian Weimer Oct. 19, 2020, 11:58 a.m. UTC | #11
* Andreas Schwab:

> On Okt 19 2020, Florian Weimer via Libc-alpha wrote:
>
>> * Adhemerval Zanella via Libc-alpha:
>>
>>> It was made deprecated on 2.31, so it moves to compat symbol after
>>> two releases.  It was also removed from exported symbol for riscv32
>>> (since ABI will be supported on for 2.33).
>>
>> Why doesn't this break the conformance tests for
>> -D_POSIX_C_SOURCE=200112L?
>>
>> Have we ever turned anything into a compatibility symbol that is
>> declared in a standard header?
>
> I think it should be handled the same way as gets.

Yes, that would make sense.  No compatibility symbol then.

Thanks,
Florian
  
Adhemerval Zanella Oct. 19, 2020, 12:30 p.m. UTC | #12
On 19/10/2020 08:58, Florian Weimer via Libc-alpha wrote:
> * Andreas Schwab:
> 
>> On Okt 19 2020, Florian Weimer via Libc-alpha wrote:
>>
>>> * Adhemerval Zanella via Libc-alpha:
>>>
>>>> It was made deprecated on 2.31, so it moves to compat symbol after
>>>> two releases.  It was also removed from exported symbol for riscv32
>>>> (since ABI will be supported on for 2.33).
>>>
>>> Why doesn't this break the conformance tests for
>>> -D_POSIX_C_SOURCE=200112L?
>>>
>>> Have we ever turned anything into a compatibility symbol that is
>>> declared in a standard header?
>>
>> I think it should be handled the same way as gets.
> 
> Yes, that would make sense.  No compatibility symbol then.

We can move this to be handled as gets and only provide the symbol
for older _POSIX_C_SOURCE source, but it will still require all the
boilerplate to provide a y2038 safe interface. I don't have a strong
preference here, but even for -D_POSIX_C_SOURCE=200112L the interface
was marked as LEGACY.
  
Adhemerval Zanella Oct. 19, 2020, 12:31 p.m. UTC | #13
On 19/10/2020 09:30, Adhemerval Zanella wrote:
> 
> 
> On 19/10/2020 08:58, Florian Weimer via Libc-alpha wrote:
>> * Andreas Schwab:
>>
>>> On Okt 19 2020, Florian Weimer via Libc-alpha wrote:
>>>
>>>> * Adhemerval Zanella via Libc-alpha:
>>>>
>>>>> It was made deprecated on 2.31, so it moves to compat symbol after
>>>>> two releases.  It was also removed from exported symbol for riscv32
>>>>> (since ABI will be supported on for 2.33).
>>>>
>>>> Why doesn't this break the conformance tests for
>>>> -D_POSIX_C_SOURCE=200112L?
>>>>
>>>> Have we ever turned anything into a compatibility symbol that is
>>>> declared in a standard header?
>>>
>>> I think it should be handled the same way as gets.
>>
>> Yes, that would make sense.  No compatibility symbol then.
> 
> We can move this to be handled as gets and only provide the symbol
> for older _POSIX_C_SOURCE source, but it will still require all the
> boilerplate to provide a y2038 safe interface. I don't have a strong
> preference here, but even for -D_POSIX_C_SOURCE=200112L the interface
> was marked as LEGACY.
> 

It seems the least disruptive way, I will send a fix.
  
Joseph Myers Oct. 19, 2020, 5:56 p.m. UTC | #14
On Mon, 19 Oct 2020, Florian Weimer via Libc-alpha wrote:

> * Adhemerval Zanella via Libc-alpha:
> 
> > It was made deprecated on 2.31, so it moves to compat symbol after
> > two releases.  It was also removed from exported symbol for riscv32
> > (since ABI will be supported on for 2.33).
> 
> Why doesn't this break the conformance tests for
> -D_POSIX_C_SOURCE=200112L?

It does break them - if you use a compiler that doesn't find a previously 
installed copy of the header (for example, rerun the compilers stage of 
build-many-glibcs.py, not just the glibcs stage).

If a required feature from an old standard is deliberately unsupported, we 
may XFAIL the tests in conform/Makefile (as done for varargs.h, for 
example).

> Have we ever turned anything into a compatibility symbol that is
> declared in a standard header?

The regexp.h obsoletion is an example.  (The header was in UNIX98, but 
marked LEGACY there, and removed in the 2001 edition of POSIX.)
  

Patch

diff --git a/include/sys/timeb.h b/include/sys/timeb.h
deleted file mode 100644
index 9f4509c35e..0000000000
--- a/include/sys/timeb.h
+++ /dev/null
@@ -1 +0,0 @@ 
-#include <time/sys/timeb.h>
diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
index 36aa34a5e7..e977715088 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
@@ -795,7 +795,6 @@  GLIBC_2.33 fsync F
 GLIBC_2.33 ftell F
 GLIBC_2.33 ftello F
 GLIBC_2.33 ftello64 F
-GLIBC_2.33 ftime F
 GLIBC_2.33 ftok F
 GLIBC_2.33 ftruncate F
 GLIBC_2.33 ftruncate64 F
diff --git a/time/Makefile b/time/Makefile
index a4fb13d6a3..ab8fb3303b 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	\
@@ -45,9 +45,10 @@  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-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
+tests-internal := tst-ftime
 
 include ../Rules
 
diff --git a/time/ftime.c b/time/ftime.c
index 93f485bbf7..be3295ef76 100644
--- a/time/ftime.c
+++ b/time/ftime.c
@@ -16,11 +16,23 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <sys/timeb.h>
+#include <shlib-compat.h>
+
+#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_33)
+
 #include <time.h>
 
+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.  */
+  };
+
 int
-ftime (struct timeb *timebuf)
+attribute_compat_text_section
+__ftime (struct timeb *timebuf)
 {
   struct timespec ts;
   __clock_gettime (CLOCK_REALTIME, &ts);
@@ -31,3 +43,6 @@  ftime (struct timeb *timebuf)
   timebuf->dstflag = 0;
   return 0;
 }
+
+compat_symbol (libc, __ftime, ftime, GLIBC_2_0);
+#endif
diff --git a/time/sys/timeb.h b/time/sys/timeb.h
deleted file mode 100644
index 641c333450..0000000000
--- a/time/sys/timeb.h
+++ /dev/null
@@ -1,44 +0,0 @@ 
-/* Copyright (C) 1994-2020 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
-   <https://www.gnu.org/licenses/>.  */
-
-#ifndef _SYS_TIMEB_H
-#define _SYS_TIMEB_H	1
-
-#include <features.h>
-
-#include <bits/types/time_t.h>
-
-__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)
-  __nonnull ((1)) __attribute_deprecated__;
-
-__END_DECLS
-
-#endif	/* sys/timeb.h */
diff --git a/time/tst-ftime.c b/time/tst-ftime.c
index 08916c0c65..6978feb0f1 100644
--- a/time/tst-ftime.c
+++ b/time/tst-ftime.c
@@ -16,9 +16,23 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <sys/timeb.h>
-#include <stdio.h>
-#include <libc-diag.h>
+
+#include <shlib-compat.h>
+#if TEST_COMPAT (libc, GLIBC_2_0, GLIBC_2_33)
+#include <time.h>
+#include <support/check.h>
+
+compat_symbol_reference (libc, ftime, ftime, GLIBC_2_0);
+
+struct timeb
+  {
+    time_t time;
+    unsigned short int millitm;
+    short int timezone;
+    short int dstflag;
+  };
+
+extern int ftime (struct timeb *__timebuf);
 
 static int
 do_test (void)
@@ -30,36 +44,23 @@  do_test (void)
     {
       prev = curr;
 
-      /* ftime was deprecated on 2.31.  */
-      DIAG_PUSH_NEEDS_COMMENT;
-      DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Wdeprecated-declarations");
-
-      if (ftime (&curr))
-        {
-          printf ("ftime returned an error\n");
-          return 1;
-        }
-
-      DIAG_POP_NEEDS_COMMENT;
-
-      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;
-        }
+      /* ftime was deprecated on 2.31 and removed on 2.33.  */
+      TEST_COMPARE (ftime (&curr), 0);
+      TEST_VERIFY_EXIT (curr.time >= prev.time);
+      if (curr.time == prev.time)
+	TEST_VERIFY_EXIT (curr.millitm >= prev.millitm);
 
       if (curr.time > prev.time)
         sec ++;
     }
   return 0;
 }
+#else
+static int
+do_test (void)
+{
+  return EXIT_UNSUPPORTED;
+}
+#endif
 
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>