On Thu, Jun 6, 2019 at 9:28 PM Florian Weimer <fweimer@xxxxxxxxxx> wrote:
>
> This regression fix still hasn't been merged into Linus' tree. What is
> going on here?
.. it was never sent to me.
That said, now that I see the patch, I wonder why we'd have that
#ifdef __KERNEL__ in here:
typedef struct {
+#ifdef __KERNEL__
int val[2];
+#else
+ int __kernel_val[2];
+#endif
} __kernel_fsid_t;
and not just unconditionally do
int __fsid_val[2]
If we're changing kernel header files, it's easy enough to change the
kernel users. I'd be more worried about user space that *uses* that
thing, and currently accesses 'val[]' by name.
So the patch looks a bit odd to me. How are people supposed to use
fsid_t if they can't look at it?
The man-page makes it pretty clear that fsid_t is complete garbage,
but it's *documented* garbage:
The f_fsid field
Solaris, Irix and POSIX have a system call statvfs(2) that
returns a struct statvfs (defined in <sys/statvfs.h>) containing an
unsigned long f_fsid. Linux, SunOS, HP-UX, 4.4BSD have a system call
statfs() that returns a struct
statfs (defined in <sys/vfs.h>) containing a fsid_t f_fsid,
where fsid_t is defined as struct { int val[2]; }. The same holds for
FreeBSD, except that it uses the include file <sys/mount.h>.
so that "val[]" name does seem to be pretty much required.
In other words, I don't think the patch is acceptable. User space sees
"val[]" and _needs_ to see it. Otherwise the type is entirely
pointless.
The proper fix is presumably do make sure the fsid_t type definitions
aren't visible to user space at all in this context, and is only
visible in <sys/statvfs.h>.
So now that I _do_ see the patch, there's no way I'll apply it.
Linus
