statx struct's stx_size pointer compatibility with uint64_t/size_t

statx struct's stx_size pointer compatibility with uint64_t/size_t

[Dominique Martinet]

Hi,

I appologize if this is not the right place to ask, I'm using the glibc
header so going here but it is just as much a linux uapi question...
Cc'd a few linux kernel folks who worked on this as well for their
opinion, any input appreciated.


A coworker ran into an incompatible-pointer-type compiler warning when
trying to pass &statxbuf.stx_size to a function expecting a size_t,
which boils down to this:
-------------
#define _GNU_SOURCE
#include <stdint.h>
#include <sys/stat.h>

static void test(uint64_t *size) {   
    (void)size;
}

int main() {   
    struct statx statxbuf;

    test(&statxbuf.stx_size);
    return 0;
}
-------------
Giving this warning:
-------------
t.c: In function ‘main’:
t.c:12:10: warning: passing argument 1 of ‘test’ from incompatible
pointer type [-Wincompatible-pointer-types]
   12 |     test(&statxbuf.stx_size);
      |          ^~~~~~~~~~~~~~~~~~
      |          |
      |          __u64 * {aka long long unsigned int *}
t.c:5:28: note: expected ‘uint64_t *’ {aka ‘long unsigned int *’} but
argument is of type ‘__u64 *’ {aka ‘long long unsigned int *’}
    5 | static void test(uint64_t *size) {
      |                  ~~~~~~~~~~^~~~
-------------
(same happens with size_t)

The final warning is probably some standard defining long and long long
cannot be treated as compatible even on archs where it is, but it's a
bit of a shame that manipulating __u64 and uint64_t yield such errors
when passing pointers around -- the types pretty much guarantee they
have to be compatible and it is just an arbitrary choice that made one
be long and the other long long on x86_64 unless I misunderstood
something?


I'm a bit at loss of what to advise here.
We need to pass the value as a pointer because it can be updated, our
use case is that the symlink size can be wrong in /proc/x/fd/ and we
will want the correct value back in a statx struct here[1].

What would be the "recommended" way of doing this?
Any chance the field could change to be uint64_t-compatible in the
future? Not sure what that implies regarding e.g. backwards
compatibility though...


[1] https://github.com/cea-hpc/robinhood/blob/1ed74893c088d78783acd2e25e8009a483510ff7/src/backends/posix.c#L248

Thanks,
-- 
Dominique

Re: statx struct's stx_size pointer compatibility with uint64_t/size_t

[Florian Weimer]

* Dominique Martinet:

> What would be the "recommended" way of doing this?
> Any chance the field could change to be uint64_t-compatible in the
> future? Not sure what that implies regarding e.g. backwards
> compatibility though...

This is a tricky subject.  We already have a copy of the type with
uint64_t fields in the installed glibc headers, but this is only used if
the kernel definition is not available.

We do not want to duplicate kernel headers too much because it causes
problems if both glibc headers and kernel headers are included in the
same translation unit.  That in turn makes it difficult to use new
kernel features by only updating the kernel headers.

I do not know why the kernel definition of __u64 does not follow that of
uint64_t in <stdint.h> (or why we even have that __u64 type), and
whether the kernel definition can be changed at this point.  We can fix
this issue with preprocessor magic, but I am not entirely sure if this
is a good idea.

If you want, you could bring this up on some kernel lists (at least
linux-api), and Cc: libc-alpha, or can do that for you.

Thanks,
Florian

Re: statx struct's stx_size pointer compatibility with uint64_t/size_t

[Dominique Martinet]

Florian Weimer wrote on Tue, Dec 17, 2019:
> We do not want to duplicate kernel headers too much because it causes
> problems if both glibc headers and kernel headers are included in the
> same translation unit.  That in turn makes it difficult to use new
> kernel features by only updating the kernel headers.

Agreed here, we would want to keep the structures compatible anyway.

> I do not know why the kernel definition of __u64 does not follow that of
> uint64_t in <stdint.h> (or why we even have that __u64 type), and
> whether the kernel definition can be changed at this point.  We can fix
> this issue with preprocessor magic, but I am not entirely sure if this
> is a good idea.
> 
> If you want, you could bring this up on some kernel lists (at least
> linux-api), and Cc: libc-alpha, or can do that for you.

Ok, I will send a follow-up to these two lists (and drop libc-help) with
content similar to my initial post.

Thank you for the fast reply!
-- 
Dominique

Re: statx struct's stx_size pointer compatibility with uint64_t/size_t

[David Howells]

Florian Weimer <fweimer@redhat.com> wrote:

> I do not know why the kernel definition of __u64 does not follow that of
> uint64_t in <stdint.h> (or why we even have that __u64 type)

Do Linux's __uXX types predate stdint.h by a few years?

> and whether the kernel definition can be changed at this point

Changing it would require an awful lot of printf format overhaul.

David

Re: statx struct's stx_size pointer compatibility with uint64_t/size_t

[David Howells]

Dominique Martinet <asmadeus@codewreck.org> wrote:

> A coworker ran into an incompatible-pointer-type compiler warning when
> trying to pass &statxbuf.stx_size to a function expecting a size_t,
> which boils down to this:

sizeof(size_t) != sizeof(__u64) on some arches, so you can't simply
interchange them.  But that's not what your example code is doing.

David

Re: statx struct's stx_size pointer compatibility with uint64_t/size_t

[Dominique Martinet]

David Howells wrote on Tue, Dec 17, 2019:
> sizeof(size_t) != sizeof(__u64) on some arches, so you can't simply
> interchange them.  But that's not what your example code is doing.

Yes, I am not too worried about size_t (I always have trouble figuring
what to expect of size_t and off_t size-wise and now statx stx_size
field being u64 doesn't help with that, but at least is clear and I have
no qualm about this -- fixed size fields are easy to think about)


I'm really thinking of this __u64 vs. uint64_t incompat. Thanks for
bringing up printf formats, userspace has PRIx64 and friends but these
are not very pretty to use and I'm sure the kernel wouldn't want these.

(Unfortunately this also pretty much means libc can't just go and change
the stdint side of things either, so I don't see any good solution to
this, but I'll let others reply on the linux-api/libc-alpha side of the
thread about this)


Thanks for the answers,
-- 
Dominique

Re: statx struct's stx_size pointer compatibility with uint64_t/size_t

[Florian Weimer]

* David Howells:

> Florian Weimer <fweimer@redhat.com> wrote:
>
>> I do not know why the kernel definition of __u64 does not follow that of
>> uint64_t in <stdint.h> (or why we even have that __u64 type)
>
> Do Linux's __uXX types predate stdint.h by a few years?

Yes, certainly __u64 predates widespread compiler support for long long.

>> and whether the kernel definition can be changed at this point
>
> Changing it would require an awful lot of printf format overhaul.

Yes, we probably should add __glibc_u64 etc. types with the same
definition as in the kernel.  For struct statx, it does not matter, but
<linux/types.h> is not namespace-clean, and that may be a problem in
other contexts.  Using __glibc_u64 etc. types consistently seems
preferable.

Thanks,
Florian