* [RFC] fanotify_event_info_fid incompatibility @ 2019-11-12 20:05 Petr Vorel 2019-11-12 20:30 ` Petr Vorel 2019-11-12 20:46 ` Szabolcs Nagy 0 siblings, 2 replies; 6+ messages in thread From: Petr Vorel @ 2019-11-12 20:05 UTC (permalink / raw) To: Rich Felker; +Cc: musl Hi Rich, musl defines struct fanotify_event_info_fid member fsid as fsid_t. This conflicts with version from Linux kernel, which defines it as __kernel_fsid_t (musl's fsid_t has int __val[2], kernel's __kernel_fsid_t has int val[2]). I see commit 32b82cf5 ("fix the fsid_t structure to match name of __val"), which looks correct to me. I also think it's wrong, that other libc (at least glibc, uclibc-ng, bionic) don't define fanotify_event_info_fid and other structs thus users are forced to use definition from <linux/fanotify.h>. But can be something done with this incompatibility? Kind regards, Petr ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] fanotify_event_info_fid incompatibility 2019-11-12 20:05 [RFC] fanotify_event_info_fid incompatibility Petr Vorel @ 2019-11-12 20:30 ` Petr Vorel 2019-11-12 20:46 ` Szabolcs Nagy 1 sibling, 0 replies; 6+ messages in thread From: Petr Vorel @ 2019-11-12 20:30 UTC (permalink / raw) To: Rich Felker; +Cc: Szabolcs Nagy, musl Hi, > musl defines struct fanotify_event_info_fid member fsid as fsid_t. This > conflicts with version from Linux kernel, which defines it as __kernel_fsid_t > (musl's fsid_t has int __val[2], kernel's __kernel_fsid_t has int val[2]). > I see commit 32b82cf5 ("fix the fsid_t structure to match name of __val"), > which looks correct to me. > I also think it's wrong, that other libc (at least glibc, uclibc-ng, bionic) > don't define fanotify_event_info_fid and other structs thus users are forced to > use definition from <linux/fanotify.h>. But can be something done with this > incompatibility? BTW this incompatibility was added quite recently, in f67b3c17 ("sys/fanotify.h: update for linux v5.1") in v1.1.23. Kind regards, Petr ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] fanotify_event_info_fid incompatibility 2019-11-12 20:05 [RFC] fanotify_event_info_fid incompatibility Petr Vorel 2019-11-12 20:30 ` Petr Vorel @ 2019-11-12 20:46 ` Szabolcs Nagy 2019-11-12 20:58 ` Petr Vorel 1 sibling, 1 reply; 6+ messages in thread From: Szabolcs Nagy @ 2019-11-12 20:46 UTC (permalink / raw) To: musl; +Cc: Rich Felker * Petr Vorel <petr.vorel@gmail.com> [2019-11-12 21:05:20 +0100]: > musl defines struct fanotify_event_info_fid member fsid as fsid_t. This > conflicts with version from Linux kernel, which defines it as __kernel_fsid_t > (musl's fsid_t has int __val[2], kernel's __kernel_fsid_t has int val[2]). > > I see commit 32b82cf5 ("fix the fsid_t structure to match name of __val"), > which looks correct to me. > > I also think it's wrong, that other libc (at least glibc, uclibc-ng, bionic) > don't define fanotify_event_info_fid and other structs thus users are forced to > use definition from <linux/fanotify.h>. But can be something done with this > incompatibility? yeah, glibc sys/fanotify.h just includes linux/fanotify.h which uses linux types instead of libc ones, this is a common pattern and there is no clean solution if users rely on that in such cases i try to keep updating sys/foo.h following linux/foo.h changes, but in musl i try to use libc types (e.g. if a field is __u64 then passing a pointer to it as uint64_t* may not be valid so the glibc way is quite problematic) glibc fsid_t uses __val but the __kernel_fsid_t has val, musl could use a different type instead of fsid_t for fanotify that has __val, but it's a bit ugly, is there a reason to access fsid_t.val? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] fanotify_event_info_fid incompatibility 2019-11-12 20:46 ` Szabolcs Nagy @ 2019-11-12 20:58 ` Petr Vorel 2019-11-12 21:29 ` Szabolcs Nagy 0 siblings, 1 reply; 6+ messages in thread From: Petr Vorel @ 2019-11-12 20:58 UTC (permalink / raw) To: musl, Rich Felker, Szabolcs Nagy Hi, > * Petr Vorel <petr.vorel@gmail.com> [2019-11-12 21:05:20 +0100]: > > musl defines struct fanotify_event_info_fid member fsid as fsid_t. This > > conflicts with version from Linux kernel, which defines it as __kernel_fsid_t > > (musl's fsid_t has int __val[2], kernel's __kernel_fsid_t has int val[2]). > > I see commit 32b82cf5 ("fix the fsid_t structure to match name of __val"), > > which looks correct to me. > > I also think it's wrong, that other libc (at least glibc, uclibc-ng, bionic) > > don't define fanotify_event_info_fid and other structs thus users are forced to > > use definition from <linux/fanotify.h>. But can be something done with this > > incompatibility? > yeah, glibc sys/fanotify.h just includes linux/fanotify.h > which uses linux types instead of libc ones, this is a > common pattern and there is no clean solution if users > rely on that > in such cases i try to keep updating sys/foo.h following > linux/foo.h changes, but in musl i try to use libc types > (e.g. if a field is __u64 then passing a pointer to it as > uint64_t* may not be valid so the glibc way is quite > problematic) Although I understand a reasons for using fsid_t instead of __kernel_fsid_t (and generally the approach of updating sys/foo.h), I still think that trying to define a kernel structure in libc but using different members isn't really user friendly :(. > glibc fsid_t uses __val but the __kernel_fsid_t has val, > musl could use a different type instead of fsid_t for > fanotify that has __val, but it's a bit ugly, is there a > reason to access fsid_t.val? LTP fanotify tests access it. Understand, that LTP is not typical user-space application (+ we can handle it either with autotools detection or just use <linux/fanotify.h> instead of <sys/fanotify.h>). Kind regards, Petr [1] https://github.com/linux-test-project/ltp/tree/master/testcases/kernel/syscalls/fanotify ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] fanotify_event_info_fid incompatibility 2019-11-12 20:58 ` Petr Vorel @ 2019-11-12 21:29 ` Szabolcs Nagy 2019-11-12 22:01 ` Petr Vorel 0 siblings, 1 reply; 6+ messages in thread From: Szabolcs Nagy @ 2019-11-12 21:29 UTC (permalink / raw) To: Petr Vorel; +Cc: musl, Rich Felker * Petr Vorel <petr.vorel@gmail.com> [2019-11-12 21:58:31 +0100]: > Hi, > > > * Petr Vorel <petr.vorel@gmail.com> [2019-11-12 21:05:20 +0100]: > > > musl defines struct fanotify_event_info_fid member fsid as fsid_t. This > > > conflicts with version from Linux kernel, which defines it as __kernel_fsid_t > > > (musl's fsid_t has int __val[2], kernel's __kernel_fsid_t has int val[2]). > > > > I see commit 32b82cf5 ("fix the fsid_t structure to match name of __val"), > > > which looks correct to me. > > > > I also think it's wrong, that other libc (at least glibc, uclibc-ng, bionic) > > > don't define fanotify_event_info_fid and other structs thus users are forced to > > > use definition from <linux/fanotify.h>. But can be something done with this > > > incompatibility? > > > yeah, glibc sys/fanotify.h just includes linux/fanotify.h > > which uses linux types instead of libc ones, this is a > > common pattern and there is no clean solution if users > > rely on that > > > in such cases i try to keep updating sys/foo.h following > > linux/foo.h changes, but in musl i try to use libc types > > (e.g. if a field is __u64 then passing a pointer to it as > > uint64_t* may not be valid so the glibc way is quite > > problematic) > > Although I understand a reasons for using fsid_t instead of __kernel_fsid_t (and > generally the approach of updating sys/foo.h), I still think that trying to > define a kernel structure in libc but using different members isn't really user > friendly :(. > > > glibc fsid_t uses __val but the __kernel_fsid_t has val, > > musl could use a different type instead of fsid_t for > > fanotify that has __val, but it's a bit ugly, is there a > > reason to access fsid_t.val? > > LTP fanotify tests access it. Understand, that LTP is not typical user-space > application (+ we can handle it either with autotools detection or just use > <linux/fanotify.h> instead of <sys/fanotify.h>). > > Kind regards, > Petr > > [1] https://github.com/linux-test-project/ltp/tree/master/testcases/kernel/syscalls/fanotify this test code has internal apis like static inline void fanotify_get_fid(const char *path, __kernel_fsid_t *fsid, struct file_handle *handle) which will not work if fsid has any other type than __kernel_fsid_t, so matching the field name is not enough, you need the linux type here. mixing linux and libc types have lot of problems, so i'd suggest to just use linux headers instead of libc ones to test this api. there are other solutions (like replicating the struct definitions with different names and memcpy the api struct to that before access), but it can't be completely clean, because there is a fundamental conflict. (unfortunately that conflict is documented: the linux man pages document the linux types as public api even though they don't mix well with libc types) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] fanotify_event_info_fid incompatibility 2019-11-12 21:29 ` Szabolcs Nagy @ 2019-11-12 22:01 ` Petr Vorel 0 siblings, 0 replies; 6+ messages in thread From: Petr Vorel @ 2019-11-12 22:01 UTC (permalink / raw) To: musl, Rich Felker, Szabolcs Nagy Hi, > > [1] https://github.com/linux-test-project/ltp/tree/master/testcases/kernel/syscalls/fanotify > this test code has internal apis like > static inline void fanotify_get_fid(const char *path, __kernel_fsid_t *fsid, > struct file_handle *handle) > which will not work if fsid has any other type than __kernel_fsid_t, > so matching the field name is not enough, you need the linux type here. Yep, that's exactly the problem. But I didn't ask to find a solution for LTP (that's not difficult), I wanted to raise the problem for musl developers to reconsider that incompatibility. The only thing I'm not sure, if any other application will want to access fsid member (searching on sources.debian.org and github.com I found only fatrace [2], but it don't touch val/__val, so probably not). > mixing linux and libc types have lot of problems, so i'd suggest to > just use linux headers instead of libc ones to test this api. > there are other solutions (like replicating the struct definitions > with different names and memcpy the api struct to that before access), > but it can't be completely clean, because there is a fundamental > conflict. (unfortunately that conflict is documented: the linux man > pages document the linux types as public api even though they don't > mix well with libc types) Yep, known problem [3]. Kind regards, Petr [2] https://sources.debian.org/src/fatrace/0.15-1/fatrace.c/?hl=149#L149 [3] https://sourceware.org/glibc/wiki/Synchronizing_Headers ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-12 22:01 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-12 20:05 [RFC] fanotify_event_info_fid incompatibility Petr Vorel 2019-11-12 20:30 ` Petr Vorel 2019-11-12 20:46 ` Szabolcs Nagy 2019-11-12 20:58 ` Petr Vorel 2019-11-12 21:29 ` Szabolcs Nagy 2019-11-12 22:01 ` Petr Vorel
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ 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 NNTP newsgroup(s).