PR ada/54040: [x32] Incorrect timeval and timespec

> >> struct timespec {
> >>         __kernel_time_t tv_sec;                 /* seconds */
> >>         long            tv_nsec;                /* nanoseconds */
> >> };
> 
> This is wrong for x32 where tv_nsec is long long, not
> long.  There are a couple places where long should be
> long long for x32.

Well yes, but the type is *not* time_t, it's conceptually a different type.

> Glibc gets them right properly:

But that's for timeval, not timespec.

> All for Linux architectures, __suseconds_t and
> __suseconds_t are the same as time_t, including
> x32.  If not, we can't use array for timeval.

Well this PR is about x32 being different from all other linux ports. There's
no guarantee that we won't have exactly the same issue on future ports where
__suseconds_t is *not* the same as time_t.

Also there's the issue of s-osinte-solaris-posix.ads which is broken with
your change right now.

The short term solution would be to apply this change:

But I'd rather settle on which type is appropriate for the tv_nsec
field before making this change.

Arno

On Mon, Nov 18, 2013 at 5:57 AM, Arnaud Charlet <charlet@adacore.com> wrote:
>> >> struct timespec {
>> >>         __kernel_time_t tv_sec;                 /* seconds */
>> >>         long            tv_nsec;                /* nanoseconds */
>> >> };
>>
>> This is wrong for x32 where tv_nsec is long long, not
>> long.  There are a couple places where long should be
>> long long for x32.
>
> Well yes, but the type is *not* time_t, it's conceptually a different type.
>
>> Glibc gets them right properly:
>
> But that's for timeval, not timespec.

Glibc has

struct timespec
  {
    __time_t tv_sec;            /* Seconds.  */
    __syscall_slong_t tv_nsec;  /* Nanoseconds.  */
  };

__syscall_slong_t is long long for x32.

>> All for Linux architectures, __suseconds_t and
>> __suseconds_t are the same as time_t, including
>> x32.  If not, we can't use array for timeval.
>
> Well this PR is about x32 being different from all other linux ports. There's
> no guarantee that we won't have exactly the same issue on future ports where
> __suseconds_t is *not* the same as time_t.

Ada was using long for time_t and

type timeval is array (1 .. 2) of C.long

It assumes that the type of tv_nsec is the same as tv_sec.

> Also there's the issue of s-osinte-solaris-posix.ads which is broken with
> your change right now.
>
> The short term solution would be to apply this change:
>
> --- s-osinte-solaris-posix.ads  (revision 298928)
> +++ s-osinte-solaris-posix.ads  (working copy)
> @@ -513,7 +513,7 @@
>
>     type timespec is record
>        tv_sec  : time_t;
> -      tv_nsec : long;
> +      tv_nsec : time_t;
>     end record;
>     pragma Convention (C, timespec);
>
> But I'd rather settle on which type is appropriate for the tv_nsec
> field before making this change.
>

__syscall_slong_t is a Linux specific type.  We
can add tv_nsec_t, which should be the same
as time_t for all the current targets.

> Ada was using long for time_t and
> 
> type timeval is array (1 .. 2) of C.long
> 
> It assumes that the type of tv_nsec is the same as tv_sec.

Yes, and that was indeed wrong/dangerous.

> > --- s-osinte-solaris-posix.ads  (revision 298928)
> > +++ s-osinte-solaris-posix.ads  (working copy)
> > @@ -513,7 +513,7 @@
> >
> >     type timespec is record
> >        tv_sec  : time_t;
> > -      tv_nsec : long;
> > +      tv_nsec : time_t;
> >     end record;
> >     pragma Convention (C, timespec);
> >
> > But I'd rather settle on which type is appropriate for the tv_nsec
> > field before making this change.
> >
> 
> __syscall_slong_t is a Linux specific type.  We
> can add tv_nsec_t, which should be the same
> as time_t for all the current targets.

Introducing tv_nsec_t looks reasonable to me.

Arno

On Mon, Nov 18, 2013 at 6:17 AM, Arnaud Charlet <charlet@adacore.com> wrote:

>> >
>> >     type timespec is record
>> >        tv_sec  : time_t;
>> > -      tv_nsec : long;
>> > +      tv_nsec : time_t;
>> >     end record;
>> >     pragma Convention (C, timespec);
>> >
>> > But I'd rather settle on which type is appropriate for the tv_nsec
>> > field before making this change.
>> >
>>
>> __syscall_slong_t is a Linux specific type.  We
>> can add tv_nsec_t, which should be the same
>> as time_t for all the current targets.
>
> Introducing tv_nsec_t looks reasonable to me.
>

Can you make the change?

Thanks.

> >> __syscall_slong_t is a Linux specific type.  We
> >> can add tv_nsec_t, which should be the same
> >> as time_t for all the current targets.
> >
> > Introducing tv_nsec_t looks reasonable to me.
> >
> 
> Can you make the change?
> 
> Thanks.

Not right now, I have lots of other things to do.

Arno