* statx struct's stx_size pointer compatibility with uint64_t/size_t [not found] <87r213aykv.fsf@oldenburg2.str.redhat.com> @ 2019-12-17 15:22 ` Dominique Martinet 2019-12-17 16:54 ` Dominique Martinet 2019-12-17 18:18 ` David Howells 0 siblings, 2 replies; 4+ messages in thread From: Dominique Martinet @ 2019-12-17 15:22 UTC (permalink / raw) To: linux-api, libc-alpha Cc: Quentin Bouget, Jeff Layton, David Howells, Florian Weimer I originally asked this on libc-help@, Florian redirected me to linux-api@ and libc-alpha@; resending my first mail and quoting his reply at the end. 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 Florian Weimer wrote on Tue, Dec 17, 2019: > * 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. Thanks, -- Dominique ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: statx struct's stx_size pointer compatibility with uint64_t/size_t 2019-12-17 15:22 ` statx struct's stx_size pointer compatibility with uint64_t/size_t Dominique Martinet @ 2019-12-17 16:54 ` Dominique Martinet 2019-12-18 11:51 ` Florian Weimer 2019-12-17 18:18 ` David Howells 1 sibling, 1 reply; 4+ messages in thread From: Dominique Martinet @ 2019-12-17 16:54 UTC (permalink / raw) To: linux-api, libc-alpha Cc: Quentin Bouget, Jeff Layton, David Howells, Florian Weimer Dominique Martinet wrote on Tue, Dec 17, 2019: > Florian Weimer wrote on Tue, Dec 17, 2019: > > 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. Looking at this from a kernel's point of view, it looks like there really was a will to simplify 64-bit ints handling over all arches and have them all define 64-bit ints as long long a few years back. See for example linux commit 0c79a8e29 ("asm/types.h: Remove include/asm-generic/int-l64.h")[1] that describes the removal of '64bit ints as long' there. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0c79a8e29b5fcbcbfd611daf9d500cfad8370fcf This makes sense to me to avoid multiplying header files for the different arches, so if anything I would be tempted to ask 'why is stdint.h uint64_t defined with just long'? -- although from what I see, musl and uClibc both also define it as just long so there must also be some logic in using the smallest possible type that fits? If someone happens to know why then perhaps the same reason could also make sense with the kernel, I don't know. Tricky, as you pointed out... (size_t is another issue and I agree it's best not to rely on it being 64 bits long anyway) Thanks, -- Dominique ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: statx struct's stx_size pointer compatibility with uint64_t/size_t 2019-12-17 16:54 ` Dominique Martinet @ 2019-12-18 11:51 ` Florian Weimer 0 siblings, 0 replies; 4+ messages in thread From: Florian Weimer @ 2019-12-18 11:51 UTC (permalink / raw) To: Dominique Martinet Cc: linux-api, libc-alpha, Quentin Bouget, Jeff Layton, David Howells * Dominique Martinet: > This makes sense to me to avoid multiplying header files for the > different arches, so if anything I would be tempted to ask 'why is > stdint.h uint64_t defined with just long'? It's not a compiler-provided header. When it was added to glibc in the 90s, I don't think long long support was universal among 64-bit compilers, and you could not just drop the type (which might have been acceptable on 32-bit architectures). Anyway, looking at this, it looks like we should define struct statx with unsigned long long int in our copy instead of uint64_t. I filed bug 25292 to track this. I guess it's just another thing to keep in mind when adding system call support to glibc headers. Thanks, Florian ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: statx struct's stx_size pointer compatibility with uint64_t/size_t 2019-12-17 15:22 ` statx struct's stx_size pointer compatibility with uint64_t/size_t Dominique Martinet 2019-12-17 16:54 ` Dominique Martinet @ 2019-12-17 18:18 ` David Howells 1 sibling, 0 replies; 4+ messages in thread From: David Howells @ 2019-12-17 18:18 UTC (permalink / raw) To: Dominique Martinet Cc: dhowells, linux-api, libc-alpha, Quentin Bouget, Jeff Layton, Florian Weimer Dominique Martinet <asmadeus@codewreck.org> wrote: > Looking at this from a kernel's point of view, it looks like there > really was a will to simplify 64-bit ints handling over all arches and > have them all define 64-bit ints as long long a few years back. Think printk() too. Do you use "%lu", "%Lu" or "%llu"? It's a lot easier if __u64 is consistently unsigned long long - then it's always "%llu". The problem with defining it as "unsigned long" on some platforms and "unsigned long long" on others is that you're guaranteed warnings on one arch or another. David ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-12-18 11:51 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <87r213aykv.fsf@oldenburg2.str.redhat.com> 2019-12-17 15:22 ` statx struct's stx_size pointer compatibility with uint64_t/size_t Dominique Martinet 2019-12-17 16:54 ` Dominique Martinet 2019-12-18 11:51 ` Florian Weimer 2019-12-17 18:18 ` David Howells
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).