Re: [musl] [PATCH] add statx

[Rich Felker <dalias@libc.org>]

On Tue, Jan 28, 2020 at 11:41:33AM +0100, Florian Weimer wrote:
> * Rich Felker:
> 
> >> >> We received feedback that our headers are not useful due to the
> >> >> __u64/uint64_t mismatch.
> >> >> 
> >> >>   <https://sourceware.org/bugzilla/show_bug.cgi?id=25292
> >> >>   <https://sourceware.org/ml/libc-alpha/2019-12/msg00631.html>
> >> >
> >> > Uhg. That change seems unfortunate since it's incompatible with
> >> > theoretical future archs having 128-bit long long -- an idea I'm not
> >> > much a fan of, but don't actually want to preclude. Is there a reason
> >> > to actually care about compatibility with the kernel types?
> >> 
> >> Yes, printf format strings. 8-(
> >
> > Why not let ppl get the warnings and fix them by casting to an
> > appropriate type to print. I prefer [u]intmax_t anyway to avoid the
> > PRIu64 etc. mess that makes your format strings unreadable and that
> > interferes with translation (*).
> 
> I'm concerned that such casts hide or even introduce bugs, like casting
> tv_sec to int.

As you've found, people will do incorrect casts regardless, in places
where it makes a bigger difference than here, where using fixed basic
types isn't an option. I don't think the solution is throwing away
proper semantic use of types.

> Here's an example from the MySQL sources:
> 
>   int my_timeval_to_str(const struct timeval *tm, char *to, uint dec)
>   {
>     int len= sprintf(to, "%d", (int) tm->tv_sec);
>     if (dec)
>       len+= my_useconds_to_str(to + len, tm->tv_usec, dec);
>     return len;
>   }

In the particular case of time, we should probably teach GCC to issue
warnings (-Wy2038?) for casts from an expression whose type was
declared via a typedef named time_t (I know GCC tracks knowledge of
the typedef name used, since it shows it in other warnings) to a type
smaller than 64-bit.

> That's why the kernel always uses unsigned long long for __u64, which
> seems a reasonable trade-off to me.  It will make porting to 128-bit
> architectures difficult.  But I think we should focus on the
> architectures we have today.

I disagree strongly with the last sentence. Designing an *API* in a
way that's not compatible with anything but long long == 64-bit is bad
API design. Arguably you could get into having something like
k_[u]intNN_t or similar, and use these, but I feel like that's just
gratuitous ugliness. The userspace type for 64-bit fixed-size fields
is [u]int64_t and we should be promoting its consistent usage, not
fragmenting things around kernel uapi mistakes. (Perhaps kernel uapi
headers should be fixed so that, when __KERNEL__ is not defined, they
include <stdint.h> and define __u64 etc. in terms of the stdint
types?)

Rich