All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Vorel <petr.vorel@gmail.com>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 2/2] fanotify: Rename fanotify_event_info_fid struct
Date: Wed, 6 Nov 2019 19:42:30 +0100	[thread overview]
Message-ID: <20191106184230.GA10470@x230> (raw)
In-Reply-To: <20191105130412.GC8511@rei.lan>

> Hi!
> > To fix build on musl, which also defines fanotify_event_info_fid,
> > but uses fsid_t type for fsid instead of __kernel_fsid_t.

> > This fixes errors:

> > fanotify13.c: In function ???do_test???:
> > fanotify13.c:278:20: error: ???fsid_t??? {aka ???struct __fsid_t???} has no member named ???val???; did you mean ???__val????
> >     event_fid->fsid.val[0],
> >                     ^~~
> > ../../../../include/tst_test.h:49:53: note: in definition of macro ???tst_res???
> >   tst_res_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__)
> >                                                      ^~~~~~~~~~~
> > fanotify13.c:279:20: error: ???fsid_t??? {aka ???struct __fsid_t???} has no member named ???val???; did you mean ???__val????
> >     event_fid->fsid.val[1],

> > musl (unlike glibc and uclibc-ng) defines fanotify_event_info_fid in
> > fanotify.h and uses fsid_t as type for fanotify_event_info_fid.fsid
> > member, which defines __val[2] (unlike val[2] in __kernel_fsid_t).

> I don't know, this really sounds like a bug in musl.
What would you propose to fix? Change fsid_t member to val[2]?
I'm a bit confused, which one is correct.
Or removing fanotify_event_info_fid struct?

In musl: 32b82cf5 ("fix the fsid_t structure to match name of __val")
changed it from val[2] to __val[2] in 2011 with comment:

    this is a case of poorly written man pages not matching the actual
    implementation, and why i hate implementing nonstandard interfaces
    with no actual documentation of how they're intended to work.

Kernel defines __kernel_fsid_t with val[2]
/* kernel include/uapi/asm-generic/posix_types.h */
#ifndef __kernel_fsid_t
typedef struct {
	int	val[2];
} __kernel_fsid_t;
#endif

And it was using val[2] at least in v2.6.31-rc1 - no sing to be __val[2].

glibc has __val[2] member. That's what triggered musl change I guess.
It looks to me it was here 2.3.1, before it used __u_quad_t
(typedef unsigned long long int __u_quad_t), but still with __val[2].

/* glibc posix/bits/types.h */
__STD_TYPE __FSID_T_TYPE __fsid_t;	/* Type of file system IDs.  */
/* glibc bits/typesizes.h */
#define __FSID_T_TYPE		struct { int __val[2]; }
/* glibc bits/statvfs.h */
struct statvfs
  {
...
    __fsid_t f_fsid;

Uclibc defines __kernel_fsid_t, sometimes defines it as val[2] and sometimes
have a condition __USE_ALL to have it val[2] (not sure if __USE_ALL is a
default, I haven't found any doc about it (is it uclibc specific or what?)

/* uclibc include/sys/types.h */
typedef __fsid_t fsid_t;

/* uclibc libc/sysdeps/linux/x86_64/bits/kernel_types.h */
typedef struct {
#ifdef __USE_ALL
	int val[2];
#else
	int __val[2];
#endif
} __kernel_fsid_t;

#endif
/* uclibc libc/sysdeps/linux/aarch64/bits/kernel_types.h */
typedef struct {
	int	val[2];
} __kernel_fsid_t;

But for __fsid_t uses the same code as glibc, with __val[2].

=> libc types use __val[2], kernel types __val.
The problem is really with mixing kernel and libc struct.
That's why I'd be to ask musl to remove it.

Kind regards,
Petr

  reply	other threads:[~2019-11-06 18:42 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-05  0:53 [LTP] [PATCH 0/2] fanotify musl fixes Petr Vorel
2019-11-05  0:53 ` [LTP] [PATCH 1/2] fanotify: Move __kernel_fsid_t definition to correct place Petr Vorel
2019-11-05 12:58   ` Cyril Hrubis
2019-11-06 18:41     ` Petr Vorel
2019-11-05  0:53 ` [LTP] [PATCH 2/2] fanotify: Rename fanotify_event_info_fid struct Petr Vorel
2019-11-05 13:04   ` Cyril Hrubis
2019-11-06 18:42     ` Petr Vorel [this message]
2019-11-05 13:11   ` Jan Stancek
2019-11-06 18:44     ` Petr Vorel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191106184230.GA10470@x230 \
    --to=petr.vorel@gmail.com \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.