[2/6] y2038: hurd: Provide __clock_settime64 function
diff mbox series

Message ID 20200118072047.23071-3-lukma@denx.de
State New
Headers show
Series
  • y2038: Convert settimeofday to be Y2038 safe
Related show

Commit Message

Lukasz Majewski Jan. 18, 2020, 7:20 a.m. UTC
For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing for
__clock_settime64 (to __clock_settime).
When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate definition
of the __clock_settime64.

The HURD port only provides __clock_settime, so this patch adds
__clock_settime64 as a tiny wrapper on it.
---
 sysdeps/mach/hurd/clock_settime.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Adhemerval Zanella Jan. 20, 2020, 5:03 p.m. UTC | #1
On 18/01/2020 04:20, Lukasz Majewski wrote:
> For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing for
> __clock_settime64 (to __clock_settime).
> When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate definition
> of the __clock_settime64.
> 
> The HURD port only provides __clock_settime, so this patch adds
> __clock_settime64 as a tiny wrapper on it.
> ---
>  sysdeps/mach/hurd/clock_settime.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/sysdeps/mach/hurd/clock_settime.c b/sysdeps/mach/hurd/clock_settime.c
> index 2c77bad71a..db1ba860dc 100644
> --- a/sysdeps/mach/hurd/clock_settime.c
> +++ b/sysdeps/mach/hurd/clock_settime.c
> @@ -53,3 +53,12 @@ versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17);
>  strong_alias (__clock_settime, __clock_settime_2);
>  compat_symbol (libc, __clock_settime_2, clock_settime, GLIBC_2_2);
>  #endif
> +
> +int
> +__clock_settime64 (clockid_t clock_id, const struct __timespec64 *ts64)
> +{
> +  struct timespec ts = valid_timespec64_to_timespec (*ts64);
> +
> +  return __clock_settime (clock_id, &ts);
> +}
> +libc_hidden_def (__clock_settime64)
> 

As for https://sourceware.org/ml/libc-alpha/2020-01/msg00445.html I don't
think this patch is really required now.
Lukasz Majewski Jan. 20, 2020, 6:08 p.m. UTC | #2
Hi Adhemerval,

> On 18/01/2020 04:20, Lukasz Majewski wrote:
> > For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing
> > for __clock_settime64 (to __clock_settime).
> > When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate
> > definition of the __clock_settime64.
> > 
> > The HURD port only provides __clock_settime, so this patch adds
> > __clock_settime64 as a tiny wrapper on it.
> > ---
> >  sysdeps/mach/hurd/clock_settime.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/sysdeps/mach/hurd/clock_settime.c
> > b/sysdeps/mach/hurd/clock_settime.c index 2c77bad71a..db1ba860dc
> > 100644 --- a/sysdeps/mach/hurd/clock_settime.c
> > +++ b/sysdeps/mach/hurd/clock_settime.c
> > @@ -53,3 +53,12 @@ versioned_symbol (libc, __clock_settime,
> > clock_settime, GLIBC_2_17); strong_alias (__clock_settime,
> > __clock_settime_2); compat_symbol (libc, __clock_settime_2,
> > clock_settime, GLIBC_2_2); #endif
> > +
> > +int
> > +__clock_settime64 (clockid_t clock_id, const struct __timespec64
> > *ts64) +{
> > +  struct timespec ts = valid_timespec64_to_timespec (*ts64);
> > +
> > +  return __clock_settime (clock_id, &ts);
> > +}
> > +libc_hidden_def (__clock_settime64)
> >   
> 
> As for https://sourceware.org/ml/libc-alpha/2020-01/msg00445.html I
> don't think this patch is really required now.

I think that it is required to allow using __clock_settime64() calls
internally in glibc.

The __clock_settime64 will be aliased to __clock_settime for archs with
__TIMESIZE==64 [*]. It is important for archs with Y2038 support - it
will allow in-glibc internal use of 64 bit time (i.e. by using struct
__timespec64).

Ports which are using Linux kernel already have it defined. The problem
is with HURD, which misses this definition.

(Please correct me if I'm wrong - but it seems to me that HURD is not
using 64 bit time?)

I thought about some kind of alias (weak, strong, #define), but the
problem was with passed data types - as __clock_settime64 requires
struct __timespec64.


Note:

[*] - __TIMESIZE indicates the default size of time_t (but on Y2038
safe ARM32 bit systems we do have __TIMESIZE=32 but we do have time_t
defined as __time64_t [1]).

Links:

[1] -
https://github.com/lmajewski/y2038_glibc/commit/e03c7188c72dd78d7141cf88f24dd2a5fd14c4f1#diff-c051022b496f12bd9028edb46b8ec04dR7




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
Adhemerval Zanella Jan. 20, 2020, 6:26 p.m. UTC | #3
On 20/01/2020 15:08, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 18/01/2020 04:20, Lukasz Majewski wrote:
>>> For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing
>>> for __clock_settime64 (to __clock_settime).
>>> When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate
>>> definition of the __clock_settime64.
>>>
>>> The HURD port only provides __clock_settime, so this patch adds
>>> __clock_settime64 as a tiny wrapper on it.
>>> ---
>>>  sysdeps/mach/hurd/clock_settime.c | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/sysdeps/mach/hurd/clock_settime.c
>>> b/sysdeps/mach/hurd/clock_settime.c index 2c77bad71a..db1ba860dc
>>> 100644 --- a/sysdeps/mach/hurd/clock_settime.c
>>> +++ b/sysdeps/mach/hurd/clock_settime.c
>>> @@ -53,3 +53,12 @@ versioned_symbol (libc, __clock_settime,
>>> clock_settime, GLIBC_2_17); strong_alias (__clock_settime,
>>> __clock_settime_2); compat_symbol (libc, __clock_settime_2,
>>> clock_settime, GLIBC_2_2); #endif
>>> +
>>> +int
>>> +__clock_settime64 (clockid_t clock_id, const struct __timespec64
>>> *ts64) +{
>>> +  struct timespec ts = valid_timespec64_to_timespec (*ts64);
>>> +
>>> +  return __clock_settime (clock_id, &ts);
>>> +}
>>> +libc_hidden_def (__clock_settime64)
>>>   
>>
>> As for https://sourceware.org/ml/libc-alpha/2020-01/msg00445.html I
>> don't think this patch is really required now.
> 
> I think that it is required to allow using __clock_settime64() calls
> internally in glibc.


Ok, I missed the patch where you adjusted the generic settimeofday.
In any case, I would like to avoid mess with generic implementation
for time64 since I am not sure how/when Hurd will try to fix it and
currently all time64 fixes are compartmentalized on Linux sysdep
folder.

I think it might be add Linux specific version of settimeofday to 
handle the time64 support.
Lukasz Majewski Jan. 21, 2020, 4:34 p.m. UTC | #4
Hi Adhemerval,

> On 20/01/2020 15:08, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 18/01/2020 04:20, Lukasz Majewski wrote:  
> >>> For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing
> >>> for __clock_settime64 (to __clock_settime).
> >>> When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate
> >>> definition of the __clock_settime64.
> >>>
> >>> The HURD port only provides __clock_settime, so this patch adds
> >>> __clock_settime64 as a tiny wrapper on it.
> >>> ---
> >>>  sysdeps/mach/hurd/clock_settime.c | 9 +++++++++
> >>>  1 file changed, 9 insertions(+)
> >>>
> >>> diff --git a/sysdeps/mach/hurd/clock_settime.c
> >>> b/sysdeps/mach/hurd/clock_settime.c index 2c77bad71a..db1ba860dc
> >>> 100644 --- a/sysdeps/mach/hurd/clock_settime.c
> >>> +++ b/sysdeps/mach/hurd/clock_settime.c
> >>> @@ -53,3 +53,12 @@ versioned_symbol (libc, __clock_settime,
> >>> clock_settime, GLIBC_2_17); strong_alias (__clock_settime,
> >>> __clock_settime_2); compat_symbol (libc, __clock_settime_2,
> >>> clock_settime, GLIBC_2_2); #endif
> >>> +
> >>> +int
> >>> +__clock_settime64 (clockid_t clock_id, const struct __timespec64
> >>> *ts64) +{
> >>> +  struct timespec ts = valid_timespec64_to_timespec (*ts64);
> >>> +
> >>> +  return __clock_settime (clock_id, &ts);
> >>> +}
> >>> +libc_hidden_def (__clock_settime64)
> >>>     
> >>
> >> As for https://sourceware.org/ml/libc-alpha/2020-01/msg00445.html I
> >> don't think this patch is really required now.  
> > 
> > I think that it is required to allow using __clock_settime64() calls
> > internally in glibc.  
> 
> 
> Ok, I missed the patch where you adjusted the generic settimeofday.
> In any case, I would like to avoid mess with generic implementation
> for time64 since I am not sure how/when Hurd will try to fix it and
> currently all time64 fixes are compartmentalized on Linux sysdep
> folder.
> 
> I think it might be add Linux specific version of settimeofday to 
> handle the time64 support.

Please correct me if I'm wrong - you suggest moving the code from this
patch (to convert settimeofday to __settimeofday64) to
sysdeps/unix/sysv/linux/settimeofday.c and do not modify
./time/settimeofday.c.

In that way there would be no need to add any code (regarding time64)
to HURD at all?


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
Adhemerval Zanella Jan. 21, 2020, 5:20 p.m. UTC | #5
On 21/01/2020 13:34, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 20/01/2020 15:08, Lukasz Majewski wrote:
>>> Hi Adhemerval,
>>>   
>>>> On 18/01/2020 04:20, Lukasz Majewski wrote:  
>>>>> For Linux glibc ports the __TIMESIZE == 64 ensures proper aliasing
>>>>> for __clock_settime64 (to __clock_settime).
>>>>> When __TIMESIZE != 64 (like ARM32, PPC) the glibc expects separate
>>>>> definition of the __clock_settime64.
>>>>>
>>>>> The HURD port only provides __clock_settime, so this patch adds
>>>>> __clock_settime64 as a tiny wrapper on it.
>>>>> ---
>>>>>  sysdeps/mach/hurd/clock_settime.c | 9 +++++++++
>>>>>  1 file changed, 9 insertions(+)
>>>>>
>>>>> diff --git a/sysdeps/mach/hurd/clock_settime.c
>>>>> b/sysdeps/mach/hurd/clock_settime.c index 2c77bad71a..db1ba860dc
>>>>> 100644 --- a/sysdeps/mach/hurd/clock_settime.c
>>>>> +++ b/sysdeps/mach/hurd/clock_settime.c
>>>>> @@ -53,3 +53,12 @@ versioned_symbol (libc, __clock_settime,
>>>>> clock_settime, GLIBC_2_17); strong_alias (__clock_settime,
>>>>> __clock_settime_2); compat_symbol (libc, __clock_settime_2,
>>>>> clock_settime, GLIBC_2_2); #endif
>>>>> +
>>>>> +int
>>>>> +__clock_settime64 (clockid_t clock_id, const struct __timespec64
>>>>> *ts64) +{
>>>>> +  struct timespec ts = valid_timespec64_to_timespec (*ts64);
>>>>> +
>>>>> +  return __clock_settime (clock_id, &ts);
>>>>> +}
>>>>> +libc_hidden_def (__clock_settime64)
>>>>>     
>>>>
>>>> As for https://sourceware.org/ml/libc-alpha/2020-01/msg00445.html I
>>>> don't think this patch is really required now.  
>>>
>>> I think that it is required to allow using __clock_settime64() calls
>>> internally in glibc.  
>>
>>
>> Ok, I missed the patch where you adjusted the generic settimeofday.
>> In any case, I would like to avoid mess with generic implementation
>> for time64 since I am not sure how/when Hurd will try to fix it and
>> currently all time64 fixes are compartmentalized on Linux sysdep
>> folder.
>>
>> I think it might be add Linux specific version of settimeofday to 
>> handle the time64 support.
> 
> Please correct me if I'm wrong - you suggest moving the code from this
> patch (to convert settimeofday to __settimeofday64) to
> sysdeps/unix/sysv/linux/settimeofday.c and do not modify
> ./time/settimeofday.c.
> 
> In that way there would be no need to add any code (regarding time64)
> to HURD at all?
> 

Yes, I would like to change the generic files for time64_t support once
Hurd wants to actually pursuit it. What I would like is to eventually
sets the generic interface to assume 64 bit time_t, as currently the
generic bits/typesizes.h still sets __TIME_T_TYPE as __SLONGWORD_TYPE.
Samuel Thibault Jan. 21, 2020, 5:22 p.m. UTC | #6
Adhemerval Zanella, le mar. 21 janv. 2020 14:20:12 -0300, a ecrit:
> On 21/01/2020 13:34, Lukasz Majewski wrote:
> > In that way there would be no need to add any code (regarding time64)
> > to HURD at all?
> 
> Yes, I would like to change the generic files for time64_t support once
> Hurd wants to actually pursuit it.

Yes, I'd like to work on it indeed.

Samuel
Samuel Thibault Jan. 27, 2020, 10:01 p.m. UTC | #7
Hello,

Adhemerval Zanella, le mar. 21 janv. 2020 14:20:12 -0300, a ecrit:
> On 21/01/2020 13:34, Lukasz Majewski wrote:
> > In that way there would be no need to add any code (regarding time64)
> > to HURD at all?
> > 
> 
> Yes, I would like to change the generic files for time64_t support once
> Hurd wants to actually pursuit it. What I would like is to eventually
> sets the generic interface to assume 64 bit time_t, as currently the
> generic bits/typesizes.h still sets __TIME_T_TYPE as __SLONGWORD_TYPE.

Yes, please do this for generic, it's simpler if we can get the
migration done together. It is fine to introduce __clock_get/settime64
which just convert between 32 and 64, and we'll plug a 64bit kernel
interface soon.

Samuel

Patch
diff mbox series

diff --git a/sysdeps/mach/hurd/clock_settime.c b/sysdeps/mach/hurd/clock_settime.c
index 2c77bad71a..db1ba860dc 100644
--- a/sysdeps/mach/hurd/clock_settime.c
+++ b/sysdeps/mach/hurd/clock_settime.c
@@ -53,3 +53,12 @@  versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17);
 strong_alias (__clock_settime, __clock_settime_2);
 compat_symbol (libc, __clock_settime_2, clock_settime, GLIBC_2_2);
 #endif
+
+int
+__clock_settime64 (clockid_t clock_id, const struct __timespec64 *ts64)
+{
+  struct timespec ts = valid_timespec64_to_timespec (*ts64);
+
+  return __clock_settime (clock_id, &ts);
+}
+libc_hidden_def (__clock_settime64)