Re: [RFC v3 3/3] sys/types.h: Make snseconds_t user visible

["Alejandro Colomar (man-pages)" <alx.manpages@gmail.com>]

Hi Adhemerval, Paul,

On 12/8/21 22:12, Adhemerval Zanella wrote:
> We can even be more clever and use something similar to what musl does:
> 
> diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h
> index 489e81136d..b962d3f95f 100644
> --- a/time/bits/types/struct_timespec.h
> +++ b/time/bits/types/struct_timespec.h
> @@ -15,19 +15,9 @@ struct timespec
>   #else
>     __time_t tv_sec;             /* Seconds.  */
>   #endif
> -#if __WORDSIZE == 64 \
> -  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \
> -  || (__TIMESIZE == 32 && !defined __USE_TIME_BITS64)
> -  __syscall_slong_t tv_nsec;   /* Nanoseconds.  */
> -#else
> -# if __BYTE_ORDER == __BIG_ENDIAN
> -  int: 32;           /* Padding.  */
> -  long int tv_nsec;  /* Nanoseconds.  */
> -# else
> -  long int tv_nsec;  /* Nanoseconds.  */
> -  int: 32;           /* Padding.  */
> -# endif
> -#endif
> +  int : 8 * (sizeof (time_t) - sizeof (long)) * (__BYTE_ORDER == __BIG_ENDIAN);
> +  long int tv_nsec;
> +  int : 8 * (sizeof (time_t) - sizeof (long)) * (__BYTE_ORDER == __LITTLE_ENDIAN);
>   };
>   
>   #endif
> 
>>
>> and report back on the minimum kernel version for which this works correctly.
> 
> The above does not show any regression on a recent 5.13, but I am not sure which
> is the minimal version it does work correctly on x32.
> 
> I don't mind bump the minimal kernel version for x32, however it would be good
> to check with x86 maintainers.

If that works, this seems to me a clean workaround to the X32 bug (IMO 
both spec and kernel bug, in others' opinions kernel bug, in others' 
opinions spec bug, but we agree there's a bug).  We simply forget the 
bug ever existed, and hope no one triggers it again in a future 
implementation.  In the manual page we can add a small bug note for past 
implementations, noting that it doesn't apply anymore; instead of a 
prominent note that some implementations are currently buggy, which 
would be much worse.

> 
>>
>> (I would still want the type changed in the standards, but only as futureproofing.)
>>

Regarding this, I completely agree with Zack, that this is a bug spec, 
and since the standards simply wrote in stone existing practice, we can 
only conclude that existing practice wasn't ideal.  Having a contract so 
specific about the type of tv_nsec is unnecessary and in general C has 
moved in the other direction: use int or typedefs almost everywhere; 
rare are the interfaces that use long (and even more rarely or never 
short), and usually justified.  This case is just an accident of history.

I also see the value of keeping some accidents just for the sake of not 
accidentally making bigger accidents, and C has traditionally done this, 
so I could live with 'long' for tv_nsec, even if it is wrong.

In this case, in principle, snseconds_t would be backwards compatible, 
and it would allow future implementations (say 100 years from now?) to 
move to int if needed (maybe for ABI stability, maybe for other future 
reasons that we cannot foresee), or would allow to keep it defined as 
long forever if API stability is god.

But if you disagree with that, the above changes seem reasonable and 
enough to me.

Paul:

On 12/8/21 19:12, Paul Eggert wrote:
 >
 > I'm still not seeing the need for a user-visible type "snseconds_t".
 >

I hope Zack and I made our reasons visible to you in our latest (and 
this) emails.

 > In response to the earlier proposal "[RFC v2 1/2] sys/types.h: Define
 > new type: snseconds_t", Joseph suggested omitting snseconds_t[1], I
 > supported this suggestion[2], and I don't recall seeing any statement
 > explaining why this type needs to be user-visible.

Joseph suggested omitting it, and also suggested that if making it 
visible, do it in a separate patch for a separate discussion, and that's 
exactly what I did.

I enjoy these discussions, even when I know beforehand that my proposed 
changes will be rejected, because I (and also the other part) usually 
learn something, and hopefully fix other (related or unrelated) things 
that help all of us.  In this case we learnt some better code from musl, 
and cleaned up some mess of code (or are in the path to do so).

So please don't feel ignored in your rejection; I took it into account :)

 >
 > This is a separate issue of whether we need a __snseconds_t type at all
 > (which is the disagreement that Rich Felker is expressing). Although the
 > type __snseconds_t may well be useful for glibc's own purposes, my
 > experience is that user code doesn't need a snseconds_t type without the
 > leading underscores. Arguably a user-visible snseconds_t type would
 > cause more confusion than it'd cure, and so would be a net negative.

Yes, and that's why in v3 they are in different patches, although since 
they have some relation, they are still in the same patch set.

Thanks,
Alex


-- 
Alejandro Colomar
Linux man-pages maintainer; https://www.kernel.org/doc/man-pages/