On Tue, Dec 26, 2023 at 02:14:38PM +0530, Disha Goel wrote:
> On 21/12/23 10:42 pm, Darrick J. Wong wrote:
>
> > On Thu, Dec 21, 2023 at 11:42:31AM +0530, Disha Goel wrote:
> > > In some distributions, __u64 is already defined in system header files,
> > > causing compilation errors when building xfstest.
> > >
> > > # make
> > > [CC] ext4_resize
> > > ext4_resize.c:17:28: error: conflicting types for '__u64'
> > > typedef unsigned long long __u64;
> > > ^~~~~
> > > In file included from /usr/include/asm/types.h:26:0,
> > > from /usr/include/linux/types.h:5,
> > > from /usr/include/linux/mount.h:4,
> > > from /usr/include/sys/mount.h:32,
> > > from ext4_resize.c:15:
> > > /usr/include/asm-generic/int-l64.h:30:23: note: previous declaration of '__u64' was here
> > > typedef unsigned long __u64;
> > > ^~~~~
> > >
> > > To address this issue, replace the custom definition of __u64 with the
> > > standard uint64_t type from <stdint.h>. uint64_t is part of the C99
> > > standard, offering a standardised approach for representing unsigned
> > > 64-bit integers. This modification enhances code consistency and
> > > ensures compatibility with standard types.
> > ...but isn't consistent with the definition in the kernel uapi headers.
> > ioctls explicitly use __u64, not uint64_t, and there are a few arches
> > (apparently) where __u64 ends up an unsigned long long but uint64_t ends
> > up an unsigned long, and the reverse.
>
> Thanks for reviewing the patch. I will take care of this and address in v2.
>
> > > Tested on various distributions on Power architecture, by successfully
> > > compiling xfstest. Additionally, verified the compatibility by running
> > > ext4/033 and ext4/056 tests, both of which use ext4_resize and
> > > observed successful test execution.
> > >
> > > # make
> > > [CC] detached_mounts_propagation
> > > [CC] ext4_resize
> > > [CC] t_readdir_3
> > >
> > > Signed-off-by: Disha Goel <disgoel@xxxxxxxxxxxxx>
> > > ---
> > > src/ext4_resize.c | 6 ++----
> > > 1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/ext4_resize.c b/src/ext4_resize.c
> > > index 78b66432..6e60ee5b 100644
> > > --- a/src/ext4_resize.c
> > > +++ b/src/ext4_resize.c
> > > @@ -14,10 +14,8 @@
> > > #include <sys/ioctl.h>
> > > #include <sys/mount.h>
> > Why not #include <linux/ext4.h>, which gets you both the __u64 typedef
> > and EXT4_IOC_RESIZE_FS?
> >
> > --D
> >
> The #include <linux/ext4.h> got added in kernel 6.4 and distros which
> I'm testing does not have <linux/ext4.h> present in kernel headers.
> We can add #include <linux/types.h> which also gets the __u64 typedef.
>
> Let me know your thoughts.
Ah, linux/ext4.h didn't exist until 6.4. Heh. Well, in the long run
fstests really ought to pull in kernel ioctl definitions whenever
possible. So:
I suppose this program should #include <linux/types.h> to get the __u64
definition, after which this can go away:
> > > -typedef unsigned long long __u64;
And then you'd need to add some m4/autoconf AC_CHECK_HEADERS magic for
configure to detect when #include <linux/ext4.h> actually results in a
compilable program; and set some make variable accordingly.
Then this program can grow a:
#ifdef HAVE_LINUX_EXT4_H
# include <linux/ext4.h>
#endif
After which this existing bit of cpp magic:
#ifndef EXT4_IOC_RESIZE_FS
# define EXT4_IOC_RESIZE_FS _IOW('f', 16, __u64)
#endif
should work for everyone.
--D
> > > -
> > > #ifndef EXT4_IOC_RESIZE_FS
> > > -#define EXT4_IOC_RESIZE_FS _IOW('f', 16, __u64)
> > > +#define EXT4_IOC_RESIZE_FS _IOW('f', 16, uint64_t)
> > > #endif
> > > #define pr_error(fmt, ...) do { \
> > > @@ -31,7 +29,7 @@ static void usage(void)
> > > int main(int argc, char **argv)
> > > {
> > > - __u64 new_size;
> > > + uint64_t new_size;
> > > int error, fd;
> > > char *mnt_dir = NULL, *tmp = NULL;
> > > --
> > > 2.39.1
> > >
> > >
>
