All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fuse: fix integer type usage in uapi header
@ 2022-03-18 17:14 Carlos Llamas
  2022-03-18 17:23 ` Masahiro Yamada
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Carlos Llamas @ 2022-03-18 17:14 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Greg Kroah-Hartman, Alessio Balsini, Masahiro Yamada,
	Arnd Bergmann, kernel-team, linux-fsdevel, linux-kernel,
	Carlos Llamas

Kernel uapi headers are supposed to use __[us]{8,16,32,64} defined by
<linux/types.h> instead of 'uint32_t' and similar. This patch changes
all the definitions in this header to use the correct type. Previous
discussion of this topic can be found here:

  https://lkml.org/lkml/2019/6/5/18

Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 include/uapi/linux/fuse.h | 509 +++++++++++++++++++-------------------
 1 file changed, 253 insertions(+), 256 deletions(-)

diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index d6ccee961891..c6dc477306c1 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -199,11 +199,7 @@
 #ifndef _LINUX_FUSE_H
 #define _LINUX_FUSE_H
 
-#ifdef __KERNEL__
 #include <linux/types.h>
-#else
-#include <stdint.h>
-#endif
 
 /*
  * Version negotiation:
@@ -238,42 +234,42 @@
    userspace works under 64bit kernels */
 
 struct fuse_attr {
-	uint64_t	ino;
-	uint64_t	size;
-	uint64_t	blocks;
-	uint64_t	atime;
-	uint64_t	mtime;
-	uint64_t	ctime;
-	uint32_t	atimensec;
-	uint32_t	mtimensec;
-	uint32_t	ctimensec;
-	uint32_t	mode;
-	uint32_t	nlink;
-	uint32_t	uid;
-	uint32_t	gid;
-	uint32_t	rdev;
-	uint32_t	blksize;
-	uint32_t	flags;
+	__u64	ino;
+	__u64	size;
+	__u64	blocks;
+	__u64	atime;
+	__u64	mtime;
+	__u64	ctime;
+	__u32	atimensec;
+	__u32	mtimensec;
+	__u32	ctimensec;
+	__u32	mode;
+	__u32	nlink;
+	__u32	uid;
+	__u32	gid;
+	__u32	rdev;
+	__u32	blksize;
+	__u32	flags;
 };
 
 struct fuse_kstatfs {
-	uint64_t	blocks;
-	uint64_t	bfree;
-	uint64_t	bavail;
-	uint64_t	files;
-	uint64_t	ffree;
-	uint32_t	bsize;
-	uint32_t	namelen;
-	uint32_t	frsize;
-	uint32_t	padding;
-	uint32_t	spare[6];
+	__u64	blocks;
+	__u64	bfree;
+	__u64	bavail;
+	__u64	files;
+	__u64	ffree;
+	__u32	bsize;
+	__u32	namelen;
+	__u32	frsize;
+	__u32	padding;
+	__u32	spare[6];
 };
 
 struct fuse_file_lock {
-	uint64_t	start;
-	uint64_t	end;
-	uint32_t	type;
-	uint32_t	pid; /* tgid */
+	__u64	start;
+	__u64	end;
+	__u32	type;
+	__u32	pid; /* tgid */
 };
 
 /**
@@ -562,149 +558,150 @@ enum fuse_notify_code {
 #define FUSE_COMPAT_ENTRY_OUT_SIZE 120
 
 struct fuse_entry_out {
-	uint64_t	nodeid;		/* Inode ID */
-	uint64_t	generation;	/* Inode generation: nodeid:gen must
-					   be unique for the fs's lifetime */
-	uint64_t	entry_valid;	/* Cache timeout for the name */
-	uint64_t	attr_valid;	/* Cache timeout for the attributes */
-	uint32_t	entry_valid_nsec;
-	uint32_t	attr_valid_nsec;
+	__u64	nodeid;		/* Inode ID */
+	__u64	generation;	/* Inode generation: nodeid:gen must
+				 * be unique for the fs's lifetime
+				 */
+	__u64	entry_valid;	/* Cache timeout for the name */
+	__u64	attr_valid;	/* Cache timeout for the attributes */
+	__u32	entry_valid_nsec;
+	__u32	attr_valid_nsec;
 	struct fuse_attr attr;
 };
 
 struct fuse_forget_in {
-	uint64_t	nlookup;
+	__u64	nlookup;
 };
 
 struct fuse_forget_one {
-	uint64_t	nodeid;
-	uint64_t	nlookup;
+	__u64	nodeid;
+	__u64	nlookup;
 };
 
 struct fuse_batch_forget_in {
-	uint32_t	count;
-	uint32_t	dummy;
+	__u32	count;
+	__u32	dummy;
 };
 
 struct fuse_getattr_in {
-	uint32_t	getattr_flags;
-	uint32_t	dummy;
-	uint64_t	fh;
+	__u32	getattr_flags;
+	__u32	dummy;
+	__u64	fh;
 };
 
 #define FUSE_COMPAT_ATTR_OUT_SIZE 96
 
 struct fuse_attr_out {
-	uint64_t	attr_valid;	/* Cache timeout for the attributes */
-	uint32_t	attr_valid_nsec;
-	uint32_t	dummy;
+	__u64	attr_valid;	/* Cache timeout for the attributes */
+	__u32	attr_valid_nsec;
+	__u32	dummy;
 	struct fuse_attr attr;
 };
 
 #define FUSE_COMPAT_MKNOD_IN_SIZE 8
 
 struct fuse_mknod_in {
-	uint32_t	mode;
-	uint32_t	rdev;
-	uint32_t	umask;
-	uint32_t	padding;
+	__u32	mode;
+	__u32	rdev;
+	__u32	umask;
+	__u32	padding;
 };
 
 struct fuse_mkdir_in {
-	uint32_t	mode;
-	uint32_t	umask;
+	__u32	mode;
+	__u32	umask;
 };
 
 struct fuse_rename_in {
-	uint64_t	newdir;
+	__u64	newdir;
 };
 
 struct fuse_rename2_in {
-	uint64_t	newdir;
-	uint32_t	flags;
-	uint32_t	padding;
+	__u64	newdir;
+	__u32	flags;
+	__u32	padding;
 };
 
 struct fuse_link_in {
-	uint64_t	oldnodeid;
+	__u64	oldnodeid;
 };
 
 struct fuse_setattr_in {
-	uint32_t	valid;
-	uint32_t	padding;
-	uint64_t	fh;
-	uint64_t	size;
-	uint64_t	lock_owner;
-	uint64_t	atime;
-	uint64_t	mtime;
-	uint64_t	ctime;
-	uint32_t	atimensec;
-	uint32_t	mtimensec;
-	uint32_t	ctimensec;
-	uint32_t	mode;
-	uint32_t	unused4;
-	uint32_t	uid;
-	uint32_t	gid;
-	uint32_t	unused5;
+	__u32	valid;
+	__u32	padding;
+	__u64	fh;
+	__u64	size;
+	__u64	lock_owner;
+	__u64	atime;
+	__u64	mtime;
+	__u64	ctime;
+	__u32	atimensec;
+	__u32	mtimensec;
+	__u32	ctimensec;
+	__u32	mode;
+	__u32	unused4;
+	__u32	uid;
+	__u32	gid;
+	__u32	unused5;
 };
 
 struct fuse_open_in {
-	uint32_t	flags;
-	uint32_t	open_flags;	/* FUSE_OPEN_... */
+	__u32	flags;
+	__u32	open_flags;	/* FUSE_OPEN_... */
 };
 
 struct fuse_create_in {
-	uint32_t	flags;
-	uint32_t	mode;
-	uint32_t	umask;
-	uint32_t	open_flags;	/* FUSE_OPEN_... */
+	__u32	flags;
+	__u32	mode;
+	__u32	umask;
+	__u32	open_flags;	/* FUSE_OPEN_... */
 };
 
 struct fuse_open_out {
-	uint64_t	fh;
-	uint32_t	open_flags;
-	uint32_t	padding;
+	__u64	fh;
+	__u32	open_flags;
+	__u32	padding;
 };
 
 struct fuse_release_in {
-	uint64_t	fh;
-	uint32_t	flags;
-	uint32_t	release_flags;
-	uint64_t	lock_owner;
+	__u64	fh;
+	__u32	flags;
+	__u32	release_flags;
+	__u64	lock_owner;
 };
 
 struct fuse_flush_in {
-	uint64_t	fh;
-	uint32_t	unused;
-	uint32_t	padding;
-	uint64_t	lock_owner;
+	__u64	fh;
+	__u32	unused;
+	__u32	padding;
+	__u64	lock_owner;
 };
 
 struct fuse_read_in {
-	uint64_t	fh;
-	uint64_t	offset;
-	uint32_t	size;
-	uint32_t	read_flags;
-	uint64_t	lock_owner;
-	uint32_t	flags;
-	uint32_t	padding;
+	__u64	fh;
+	__u64	offset;
+	__u32	size;
+	__u32	read_flags;
+	__u64	lock_owner;
+	__u32	flags;
+	__u32	padding;
 };
 
 #define FUSE_COMPAT_WRITE_IN_SIZE 24
 
 struct fuse_write_in {
-	uint64_t	fh;
-	uint64_t	offset;
-	uint32_t	size;
-	uint32_t	write_flags;
-	uint64_t	lock_owner;
-	uint32_t	flags;
-	uint32_t	padding;
+	__u64	fh;
+	__u64	offset;
+	__u32	size;
+	__u32	write_flags;
+	__u64	lock_owner;
+	__u32	flags;
+	__u32	padding;
 };
 
 struct fuse_write_out {
-	uint32_t	size;
-	uint32_t	padding;
+	__u32	size;
+	__u32	padding;
 };
 
 #define FUSE_COMPAT_STATFS_SIZE 48
@@ -714,36 +711,36 @@ struct fuse_statfs_out {
 };
 
 struct fuse_fsync_in {
-	uint64_t	fh;
-	uint32_t	fsync_flags;
-	uint32_t	padding;
+	__u64	fh;
+	__u32	fsync_flags;
+	__u32	padding;
 };
 
 #define FUSE_COMPAT_SETXATTR_IN_SIZE 8
 
 struct fuse_setxattr_in {
-	uint32_t	size;
-	uint32_t	flags;
-	uint32_t	setxattr_flags;
-	uint32_t	padding;
+	__u32	size;
+	__u32	flags;
+	__u32	setxattr_flags;
+	__u32	padding;
 };
 
 struct fuse_getxattr_in {
-	uint32_t	size;
-	uint32_t	padding;
+	__u32	size;
+	__u32	padding;
 };
 
 struct fuse_getxattr_out {
-	uint32_t	size;
-	uint32_t	padding;
+	__u32	size;
+	__u32	padding;
 };
 
 struct fuse_lk_in {
-	uint64_t	fh;
-	uint64_t	owner;
+	__u64	fh;
+	__u64	owner;
 	struct fuse_file_lock lk;
-	uint32_t	lk_flags;
-	uint32_t	padding;
+	__u32	lk_flags;
+	__u32	padding;
 };
 
 struct fuse_lk_out {
@@ -751,145 +748,145 @@ struct fuse_lk_out {
 };
 
 struct fuse_access_in {
-	uint32_t	mask;
-	uint32_t	padding;
+	__u32	mask;
+	__u32	padding;
 };
 
 struct fuse_init_in {
-	uint32_t	major;
-	uint32_t	minor;
-	uint32_t	max_readahead;
-	uint32_t	flags;
-	uint32_t	flags2;
-	uint32_t	unused[11];
+	__u32	major;
+	__u32	minor;
+	__u32	max_readahead;
+	__u32	flags;
+	__u32	flags2;
+	__u32	unused[11];
 };
 
 #define FUSE_COMPAT_INIT_OUT_SIZE 8
 #define FUSE_COMPAT_22_INIT_OUT_SIZE 24
 
 struct fuse_init_out {
-	uint32_t	major;
-	uint32_t	minor;
-	uint32_t	max_readahead;
-	uint32_t	flags;
-	uint16_t	max_background;
-	uint16_t	congestion_threshold;
-	uint32_t	max_write;
-	uint32_t	time_gran;
-	uint16_t	max_pages;
-	uint16_t	map_alignment;
-	uint32_t	flags2;
-	uint32_t	unused[7];
+	__u32	major;
+	__u32	minor;
+	__u32	max_readahead;
+	__u32	flags;
+	__u16	max_background;
+	__u16	congestion_threshold;
+	__u32	max_write;
+	__u32	time_gran;
+	__u16	max_pages;
+	__u16	map_alignment;
+	__u32	flags2;
+	__u32	unused[7];
 };
 
 #define CUSE_INIT_INFO_MAX 4096
 
 struct cuse_init_in {
-	uint32_t	major;
-	uint32_t	minor;
-	uint32_t	unused;
-	uint32_t	flags;
+	__u32	major;
+	__u32	minor;
+	__u32	unused;
+	__u32	flags;
 };
 
 struct cuse_init_out {
-	uint32_t	major;
-	uint32_t	minor;
-	uint32_t	unused;
-	uint32_t	flags;
-	uint32_t	max_read;
-	uint32_t	max_write;
-	uint32_t	dev_major;		/* chardev major */
-	uint32_t	dev_minor;		/* chardev minor */
-	uint32_t	spare[10];
+	__u32	major;
+	__u32	minor;
+	__u32	unused;
+	__u32	flags;
+	__u32	max_read;
+	__u32	max_write;
+	__u32	dev_major;		/* chardev major */
+	__u32	dev_minor;		/* chardev minor */
+	__u32	spare[10];
 };
 
 struct fuse_interrupt_in {
-	uint64_t	unique;
+	__u64	unique;
 };
 
 struct fuse_bmap_in {
-	uint64_t	block;
-	uint32_t	blocksize;
-	uint32_t	padding;
+	__u64	block;
+	__u32	blocksize;
+	__u32	padding;
 };
 
 struct fuse_bmap_out {
-	uint64_t	block;
+	__u64	block;
 };
 
 struct fuse_ioctl_in {
-	uint64_t	fh;
-	uint32_t	flags;
-	uint32_t	cmd;
-	uint64_t	arg;
-	uint32_t	in_size;
-	uint32_t	out_size;
+	__u64	fh;
+	__u32	flags;
+	__u32	cmd;
+	__u64	arg;
+	__u32	in_size;
+	__u32	out_size;
 };
 
 struct fuse_ioctl_iovec {
-	uint64_t	base;
-	uint64_t	len;
+	__u64	base;
+	__u64	len;
 };
 
 struct fuse_ioctl_out {
-	int32_t		result;
-	uint32_t	flags;
-	uint32_t	in_iovs;
-	uint32_t	out_iovs;
+	__s32	result;
+	__u32	flags;
+	__u32	in_iovs;
+	__u32	out_iovs;
 };
 
 struct fuse_poll_in {
-	uint64_t	fh;
-	uint64_t	kh;
-	uint32_t	flags;
-	uint32_t	events;
+	__u64	fh;
+	__u64	kh;
+	__u32	flags;
+	__u32	events;
 };
 
 struct fuse_poll_out {
-	uint32_t	revents;
-	uint32_t	padding;
+	__u32	revents;
+	__u32	padding;
 };
 
 struct fuse_notify_poll_wakeup_out {
-	uint64_t	kh;
+	__u64	kh;
 };
 
 struct fuse_fallocate_in {
-	uint64_t	fh;
-	uint64_t	offset;
-	uint64_t	length;
-	uint32_t	mode;
-	uint32_t	padding;
+	__u64	fh;
+	__u64	offset;
+	__u64	length;
+	__u32	mode;
+	__u32	padding;
 };
 
 struct fuse_in_header {
-	uint32_t	len;
-	uint32_t	opcode;
-	uint64_t	unique;
-	uint64_t	nodeid;
-	uint32_t	uid;
-	uint32_t	gid;
-	uint32_t	pid;
-	uint32_t	padding;
+	__u32	len;
+	__u32	opcode;
+	__u64	unique;
+	__u64	nodeid;
+	__u32	uid;
+	__u32	gid;
+	__u32	pid;
+	__u32	padding;
 };
 
 struct fuse_out_header {
-	uint32_t	len;
-	int32_t		error;
-	uint64_t	unique;
+	__u32	len;
+	__s32		error;
+	__u64	unique;
 };
 
 struct fuse_dirent {
-	uint64_t	ino;
-	uint64_t	off;
-	uint32_t	namelen;
-	uint32_t	type;
+	__u64	ino;
+	__u64	off;
+	__u32	namelen;
+	__u32	type;
 	char name[];
 };
 
 /* Align variable length records to 64bit boundary */
 #define FUSE_REC_ALIGN(x) \
-	(((x) + sizeof(uint64_t) - 1) & ~(sizeof(uint64_t) - 1))
+	(((x) + sizeof(__u64) - 1) & ~(sizeof(__u64) - 1))
 
 #define FUSE_NAME_OFFSET offsetof(struct fuse_dirent, name)
 #define FUSE_DIRENT_ALIGN(x) FUSE_REC_ALIGN(x)
@@ -907,106 +904,106 @@ struct fuse_direntplus {
 	FUSE_DIRENT_ALIGN(FUSE_NAME_OFFSET_DIRENTPLUS + (d)->dirent.namelen)
 
 struct fuse_notify_inval_inode_out {
-	uint64_t	ino;
-	int64_t		off;
-	int64_t		len;
+	__u64	ino;
+	__s64	off;
+	__s64	len;
 };
 
 struct fuse_notify_inval_entry_out {
-	uint64_t	parent;
-	uint32_t	namelen;
-	uint32_t	padding;
+	__u64	parent;
+	__u32	namelen;
+	__u32	padding;
 };
 
 struct fuse_notify_delete_out {
-	uint64_t	parent;
-	uint64_t	child;
-	uint32_t	namelen;
-	uint32_t	padding;
+	__u64	parent;
+	__u64	child;
+	__u32	namelen;
+	__u32	padding;
 };
 
 struct fuse_notify_store_out {
-	uint64_t	nodeid;
-	uint64_t	offset;
-	uint32_t	size;
-	uint32_t	padding;
+	__u64	nodeid;
+	__u64	offset;
+	__u32	size;
+	__u32	padding;
 };
 
 struct fuse_notify_retrieve_out {
-	uint64_t	notify_unique;
-	uint64_t	nodeid;
-	uint64_t	offset;
-	uint32_t	size;
-	uint32_t	padding;
+	__u64	notify_unique;
+	__u64	nodeid;
+	__u64	offset;
+	__u32	size;
+	__u32	padding;
 };
 
 /* Matches the size of fuse_write_in */
 struct fuse_notify_retrieve_in {
-	uint64_t	dummy1;
-	uint64_t	offset;
-	uint32_t	size;
-	uint32_t	dummy2;
-	uint64_t	dummy3;
-	uint64_t	dummy4;
+	__u64	dummy1;
+	__u64	offset;
+	__u32	size;
+	__u32	dummy2;
+	__u64	dummy3;
+	__u64	dummy4;
 };
 
 /* Device ioctls: */
 #define FUSE_DEV_IOC_MAGIC		229
-#define FUSE_DEV_IOC_CLONE		_IOR(FUSE_DEV_IOC_MAGIC, 0, uint32_t)
+#define FUSE_DEV_IOC_CLONE		_IOR(FUSE_DEV_IOC_MAGIC, 0, __u32)
 
 struct fuse_lseek_in {
-	uint64_t	fh;
-	uint64_t	offset;
-	uint32_t	whence;
-	uint32_t	padding;
+	__u64	fh;
+	__u64	offset;
+	__u32	whence;
+	__u32	padding;
 };
 
 struct fuse_lseek_out {
-	uint64_t	offset;
+	__u64	offset;
 };
 
 struct fuse_copy_file_range_in {
-	uint64_t	fh_in;
-	uint64_t	off_in;
-	uint64_t	nodeid_out;
-	uint64_t	fh_out;
-	uint64_t	off_out;
-	uint64_t	len;
-	uint64_t	flags;
+	__u64	fh_in;
+	__u64	off_in;
+	__u64	nodeid_out;
+	__u64	fh_out;
+	__u64	off_out;
+	__u64	len;
+	__u64	flags;
 };
 
 #define FUSE_SETUPMAPPING_FLAG_WRITE (1ull << 0)
 #define FUSE_SETUPMAPPING_FLAG_READ (1ull << 1)
 struct fuse_setupmapping_in {
 	/* An already open handle */
-	uint64_t	fh;
+	__u64	fh;
 	/* Offset into the file to start the mapping */
-	uint64_t	foffset;
+	__u64	foffset;
 	/* Length of mapping required */
-	uint64_t	len;
+	__u64	len;
 	/* Flags, FUSE_SETUPMAPPING_FLAG_* */
-	uint64_t	flags;
+	__u64	flags;
 	/* Offset in Memory Window */
-	uint64_t	moffset;
+	__u64	moffset;
 };
 
 struct fuse_removemapping_in {
 	/* number of fuse_removemapping_one follows */
-	uint32_t        count;
+	__u32	count;
 };
 
 struct fuse_removemapping_one {
 	/* Offset into the dax window start the unmapping */
-	uint64_t        moffset;
+	__u64	moffset;
 	/* Length of mapping required */
-	uint64_t	len;
+	__u64	len;
 };
 
 #define FUSE_REMOVEMAPPING_MAX_ENTRY   \
 		(PAGE_SIZE / sizeof(struct fuse_removemapping_one))
 
 struct fuse_syncfs_in {
-	uint64_t	padding;
+	__u64	padding;
 };
 
 /*
@@ -1016,8 +1013,8 @@ struct fuse_syncfs_in {
  * fuse_secctx, name, context
  */
 struct fuse_secctx {
-	uint32_t	size;
-	uint32_t	padding;
+	__u32	size;
+	__u32	padding;
 };
 
 /*
@@ -1027,8 +1024,8 @@ struct fuse_secctx {
  *
  */
 struct fuse_secctx_header {
-	uint32_t	size;
-	uint32_t	nr_secctx;
+	__u32	size;
+	__u32	nr_secctx;
 };
 
 #endif /* _LINUX_FUSE_H */
-- 
2.35.1.894.gb6a874cedc-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] fuse: fix integer type usage in uapi header
  2022-03-18 17:14 [PATCH] fuse: fix integer type usage in uapi header Carlos Llamas
@ 2022-03-18 17:23 ` Masahiro Yamada
  2022-03-18 19:24 ` Miklos Szeredi
  2022-03-19  6:40 ` Greg Kroah-Hartman
  2 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-03-18 17:23 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Miklos Szeredi, Greg Kroah-Hartman, Alessio Balsini,
	Arnd Bergmann, Cc: Android Kernel, Linux FS-devel Mailing List,
	Linux Kernel Mailing List

On Sat, Mar 19, 2022 at 2:14 AM Carlos Llamas <cmllamas@google.com> wrote:
>
> Kernel uapi headers are supposed to use __[us]{8,16,32,64} defined by
> <linux/types.h> instead of 'uint32_t' and similar. This patch changes
> all the definitions in this header to use the correct type. Previous
> discussion of this topic can be found here:
>
>   https://lkml.org/lkml/2019/6/5/18
>
> Signed-off-by: Carlos Llamas <cmllamas@google.com>


Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>

You can fix include/uapi/linux/idxd.h as well if you are interested.






> ---
>  include/uapi/linux/fuse.h | 509 +++++++++++++++++++-------------------
>  1 file changed, 253 insertions(+), 256 deletions(-)
>
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index d6ccee961891..c6dc477306c1 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -199,11 +199,7 @@
>  #ifndef _LINUX_FUSE_H
>  #define _LINUX_FUSE_H
>
> -#ifdef __KERNEL__
>  #include <linux/types.h>
> -#else
> -#include <stdint.h>
> -#endif
>
>  /*
>   * Version negotiation:
> @@ -238,42 +234,42 @@
>     userspace works under 64bit kernels */
>
>  struct fuse_attr {
> -       uint64_t        ino;
> -       uint64_t        size;
> -       uint64_t        blocks;
> -       uint64_t        atime;
> -       uint64_t        mtime;
> -       uint64_t        ctime;
> -       uint32_t        atimensec;
> -       uint32_t        mtimensec;
> -       uint32_t        ctimensec;
> -       uint32_t        mode;
> -       uint32_t        nlink;
> -       uint32_t        uid;
> -       uint32_t        gid;
> -       uint32_t        rdev;
> -       uint32_t        blksize;
> -       uint32_t        flags;
> +       __u64   ino;
> +       __u64   size;
> +       __u64   blocks;
> +       __u64   atime;
> +       __u64   mtime;
> +       __u64   ctime;
> +       __u32   atimensec;
> +       __u32   mtimensec;
> +       __u32   ctimensec;
> +       __u32   mode;
> +       __u32   nlink;
> +       __u32   uid;
> +       __u32   gid;
> +       __u32   rdev;
> +       __u32   blksize;
> +       __u32   flags;
>  };
>
>  struct fuse_kstatfs {
> -       uint64_t        blocks;
> -       uint64_t        bfree;
> -       uint64_t        bavail;
> -       uint64_t        files;
> -       uint64_t        ffree;
> -       uint32_t        bsize;
> -       uint32_t        namelen;
> -       uint32_t        frsize;
> -       uint32_t        padding;
> -       uint32_t        spare[6];
> +       __u64   blocks;
> +       __u64   bfree;
> +       __u64   bavail;
> +       __u64   files;
> +       __u64   ffree;
> +       __u32   bsize;
> +       __u32   namelen;
> +       __u32   frsize;
> +       __u32   padding;
> +       __u32   spare[6];
>  };
>
>  struct fuse_file_lock {
> -       uint64_t        start;
> -       uint64_t        end;
> -       uint32_t        type;
> -       uint32_t        pid; /* tgid */
> +       __u64   start;
> +       __u64   end;
> +       __u32   type;
> +       __u32   pid; /* tgid */
>  };
>
>  /**
> @@ -562,149 +558,150 @@ enum fuse_notify_code {
>  #define FUSE_COMPAT_ENTRY_OUT_SIZE 120
>
>  struct fuse_entry_out {
> -       uint64_t        nodeid;         /* Inode ID */
> -       uint64_t        generation;     /* Inode generation: nodeid:gen must
> -                                          be unique for the fs's lifetime */
> -       uint64_t        entry_valid;    /* Cache timeout for the name */
> -       uint64_t        attr_valid;     /* Cache timeout for the attributes */
> -       uint32_t        entry_valid_nsec;
> -       uint32_t        attr_valid_nsec;
> +       __u64   nodeid;         /* Inode ID */
> +       __u64   generation;     /* Inode generation: nodeid:gen must
> +                                * be unique for the fs's lifetime
> +                                */
> +       __u64   entry_valid;    /* Cache timeout for the name */
> +       __u64   attr_valid;     /* Cache timeout for the attributes */
> +       __u32   entry_valid_nsec;
> +       __u32   attr_valid_nsec;
>         struct fuse_attr attr;
>  };
>
>  struct fuse_forget_in {
> -       uint64_t        nlookup;
> +       __u64   nlookup;
>  };
>
>  struct fuse_forget_one {
> -       uint64_t        nodeid;
> -       uint64_t        nlookup;
> +       __u64   nodeid;
> +       __u64   nlookup;
>  };
>
>  struct fuse_batch_forget_in {
> -       uint32_t        count;
> -       uint32_t        dummy;
> +       __u32   count;
> +       __u32   dummy;
>  };
>
>  struct fuse_getattr_in {
> -       uint32_t        getattr_flags;
> -       uint32_t        dummy;
> -       uint64_t        fh;
> +       __u32   getattr_flags;
> +       __u32   dummy;
> +       __u64   fh;
>  };
>
>  #define FUSE_COMPAT_ATTR_OUT_SIZE 96
>
>  struct fuse_attr_out {
> -       uint64_t        attr_valid;     /* Cache timeout for the attributes */
> -       uint32_t        attr_valid_nsec;
> -       uint32_t        dummy;
> +       __u64   attr_valid;     /* Cache timeout for the attributes */
> +       __u32   attr_valid_nsec;
> +       __u32   dummy;
>         struct fuse_attr attr;
>  };
>
>  #define FUSE_COMPAT_MKNOD_IN_SIZE 8
>
>  struct fuse_mknod_in {
> -       uint32_t        mode;
> -       uint32_t        rdev;
> -       uint32_t        umask;
> -       uint32_t        padding;
> +       __u32   mode;
> +       __u32   rdev;
> +       __u32   umask;
> +       __u32   padding;
>  };
>
>  struct fuse_mkdir_in {
> -       uint32_t        mode;
> -       uint32_t        umask;
> +       __u32   mode;
> +       __u32   umask;
>  };
>
>  struct fuse_rename_in {
> -       uint64_t        newdir;
> +       __u64   newdir;
>  };
>
>  struct fuse_rename2_in {
> -       uint64_t        newdir;
> -       uint32_t        flags;
> -       uint32_t        padding;
> +       __u64   newdir;
> +       __u32   flags;
> +       __u32   padding;
>  };
>
>  struct fuse_link_in {
> -       uint64_t        oldnodeid;
> +       __u64   oldnodeid;
>  };
>
>  struct fuse_setattr_in {
> -       uint32_t        valid;
> -       uint32_t        padding;
> -       uint64_t        fh;
> -       uint64_t        size;
> -       uint64_t        lock_owner;
> -       uint64_t        atime;
> -       uint64_t        mtime;
> -       uint64_t        ctime;
> -       uint32_t        atimensec;
> -       uint32_t        mtimensec;
> -       uint32_t        ctimensec;
> -       uint32_t        mode;
> -       uint32_t        unused4;
> -       uint32_t        uid;
> -       uint32_t        gid;
> -       uint32_t        unused5;
> +       __u32   valid;
> +       __u32   padding;
> +       __u64   fh;
> +       __u64   size;
> +       __u64   lock_owner;
> +       __u64   atime;
> +       __u64   mtime;
> +       __u64   ctime;
> +       __u32   atimensec;
> +       __u32   mtimensec;
> +       __u32   ctimensec;
> +       __u32   mode;
> +       __u32   unused4;
> +       __u32   uid;
> +       __u32   gid;
> +       __u32   unused5;
>  };
>
>  struct fuse_open_in {
> -       uint32_t        flags;
> -       uint32_t        open_flags;     /* FUSE_OPEN_... */
> +       __u32   flags;
> +       __u32   open_flags;     /* FUSE_OPEN_... */
>  };
>
>  struct fuse_create_in {
> -       uint32_t        flags;
> -       uint32_t        mode;
> -       uint32_t        umask;
> -       uint32_t        open_flags;     /* FUSE_OPEN_... */
> +       __u32   flags;
> +       __u32   mode;
> +       __u32   umask;
> +       __u32   open_flags;     /* FUSE_OPEN_... */
>  };
>
>  struct fuse_open_out {
> -       uint64_t        fh;
> -       uint32_t        open_flags;
> -       uint32_t        padding;
> +       __u64   fh;
> +       __u32   open_flags;
> +       __u32   padding;
>  };
>
>  struct fuse_release_in {
> -       uint64_t        fh;
> -       uint32_t        flags;
> -       uint32_t        release_flags;
> -       uint64_t        lock_owner;
> +       __u64   fh;
> +       __u32   flags;
> +       __u32   release_flags;
> +       __u64   lock_owner;
>  };
>
>  struct fuse_flush_in {
> -       uint64_t        fh;
> -       uint32_t        unused;
> -       uint32_t        padding;
> -       uint64_t        lock_owner;
> +       __u64   fh;
> +       __u32   unused;
> +       __u32   padding;
> +       __u64   lock_owner;
>  };
>
>  struct fuse_read_in {
> -       uint64_t        fh;
> -       uint64_t        offset;
> -       uint32_t        size;
> -       uint32_t        read_flags;
> -       uint64_t        lock_owner;
> -       uint32_t        flags;
> -       uint32_t        padding;
> +       __u64   fh;
> +       __u64   offset;
> +       __u32   size;
> +       __u32   read_flags;
> +       __u64   lock_owner;
> +       __u32   flags;
> +       __u32   padding;
>  };
>
>  #define FUSE_COMPAT_WRITE_IN_SIZE 24
>
>  struct fuse_write_in {
> -       uint64_t        fh;
> -       uint64_t        offset;
> -       uint32_t        size;
> -       uint32_t        write_flags;
> -       uint64_t        lock_owner;
> -       uint32_t        flags;
> -       uint32_t        padding;
> +       __u64   fh;
> +       __u64   offset;
> +       __u32   size;
> +       __u32   write_flags;
> +       __u64   lock_owner;
> +       __u32   flags;
> +       __u32   padding;
>  };
>
>  struct fuse_write_out {
> -       uint32_t        size;
> -       uint32_t        padding;
> +       __u32   size;
> +       __u32   padding;
>  };
>
>  #define FUSE_COMPAT_STATFS_SIZE 48
> @@ -714,36 +711,36 @@ struct fuse_statfs_out {
>  };
>
>  struct fuse_fsync_in {
> -       uint64_t        fh;
> -       uint32_t        fsync_flags;
> -       uint32_t        padding;
> +       __u64   fh;
> +       __u32   fsync_flags;
> +       __u32   padding;
>  };
>
>  #define FUSE_COMPAT_SETXATTR_IN_SIZE 8
>
>  struct fuse_setxattr_in {
> -       uint32_t        size;
> -       uint32_t        flags;
> -       uint32_t        setxattr_flags;
> -       uint32_t        padding;
> +       __u32   size;
> +       __u32   flags;
> +       __u32   setxattr_flags;
> +       __u32   padding;
>  };
>
>  struct fuse_getxattr_in {
> -       uint32_t        size;
> -       uint32_t        padding;
> +       __u32   size;
> +       __u32   padding;
>  };
>
>  struct fuse_getxattr_out {
> -       uint32_t        size;
> -       uint32_t        padding;
> +       __u32   size;
> +       __u32   padding;
>  };
>
>  struct fuse_lk_in {
> -       uint64_t        fh;
> -       uint64_t        owner;
> +       __u64   fh;
> +       __u64   owner;
>         struct fuse_file_lock lk;
> -       uint32_t        lk_flags;
> -       uint32_t        padding;
> +       __u32   lk_flags;
> +       __u32   padding;
>  };
>
>  struct fuse_lk_out {
> @@ -751,145 +748,145 @@ struct fuse_lk_out {
>  };
>
>  struct fuse_access_in {
> -       uint32_t        mask;
> -       uint32_t        padding;
> +       __u32   mask;
> +       __u32   padding;
>  };
>
>  struct fuse_init_in {
> -       uint32_t        major;
> -       uint32_t        minor;
> -       uint32_t        max_readahead;
> -       uint32_t        flags;
> -       uint32_t        flags2;
> -       uint32_t        unused[11];
> +       __u32   major;
> +       __u32   minor;
> +       __u32   max_readahead;
> +       __u32   flags;
> +       __u32   flags2;
> +       __u32   unused[11];
>  };
>
>  #define FUSE_COMPAT_INIT_OUT_SIZE 8
>  #define FUSE_COMPAT_22_INIT_OUT_SIZE 24
>
>  struct fuse_init_out {
> -       uint32_t        major;
> -       uint32_t        minor;
> -       uint32_t        max_readahead;
> -       uint32_t        flags;
> -       uint16_t        max_background;
> -       uint16_t        congestion_threshold;
> -       uint32_t        max_write;
> -       uint32_t        time_gran;
> -       uint16_t        max_pages;
> -       uint16_t        map_alignment;
> -       uint32_t        flags2;
> -       uint32_t        unused[7];
> +       __u32   major;
> +       __u32   minor;
> +       __u32   max_readahead;
> +       __u32   flags;
> +       __u16   max_background;
> +       __u16   congestion_threshold;
> +       __u32   max_write;
> +       __u32   time_gran;
> +       __u16   max_pages;
> +       __u16   map_alignment;
> +       __u32   flags2;
> +       __u32   unused[7];
>  };
>
>  #define CUSE_INIT_INFO_MAX 4096
>
>  struct cuse_init_in {
> -       uint32_t        major;
> -       uint32_t        minor;
> -       uint32_t        unused;
> -       uint32_t        flags;
> +       __u32   major;
> +       __u32   minor;
> +       __u32   unused;
> +       __u32   flags;
>  };
>
>  struct cuse_init_out {
> -       uint32_t        major;
> -       uint32_t        minor;
> -       uint32_t        unused;
> -       uint32_t        flags;
> -       uint32_t        max_read;
> -       uint32_t        max_write;
> -       uint32_t        dev_major;              /* chardev major */
> -       uint32_t        dev_minor;              /* chardev minor */
> -       uint32_t        spare[10];
> +       __u32   major;
> +       __u32   minor;
> +       __u32   unused;
> +       __u32   flags;
> +       __u32   max_read;
> +       __u32   max_write;
> +       __u32   dev_major;              /* chardev major */
> +       __u32   dev_minor;              /* chardev minor */
> +       __u32   spare[10];
>  };
>
>  struct fuse_interrupt_in {
> -       uint64_t        unique;
> +       __u64   unique;
>  };
>
>  struct fuse_bmap_in {
> -       uint64_t        block;
> -       uint32_t        blocksize;
> -       uint32_t        padding;
> +       __u64   block;
> +       __u32   blocksize;
> +       __u32   padding;
>  };
>
>  struct fuse_bmap_out {
> -       uint64_t        block;
> +       __u64   block;
>  };
>
>  struct fuse_ioctl_in {
> -       uint64_t        fh;
> -       uint32_t        flags;
> -       uint32_t        cmd;
> -       uint64_t        arg;
> -       uint32_t        in_size;
> -       uint32_t        out_size;
> +       __u64   fh;
> +       __u32   flags;
> +       __u32   cmd;
> +       __u64   arg;
> +       __u32   in_size;
> +       __u32   out_size;
>  };
>
>  struct fuse_ioctl_iovec {
> -       uint64_t        base;
> -       uint64_t        len;
> +       __u64   base;
> +       __u64   len;
>  };
>
>  struct fuse_ioctl_out {
> -       int32_t         result;
> -       uint32_t        flags;
> -       uint32_t        in_iovs;
> -       uint32_t        out_iovs;
> +       __s32   result;
> +       __u32   flags;
> +       __u32   in_iovs;
> +       __u32   out_iovs;
>  };
>
>  struct fuse_poll_in {
> -       uint64_t        fh;
> -       uint64_t        kh;
> -       uint32_t        flags;
> -       uint32_t        events;
> +       __u64   fh;
> +       __u64   kh;
> +       __u32   flags;
> +       __u32   events;
>  };
>
>  struct fuse_poll_out {
> -       uint32_t        revents;
> -       uint32_t        padding;
> +       __u32   revents;
> +       __u32   padding;
>  };
>
>  struct fuse_notify_poll_wakeup_out {
> -       uint64_t        kh;
> +       __u64   kh;
>  };
>
>  struct fuse_fallocate_in {
> -       uint64_t        fh;
> -       uint64_t        offset;
> -       uint64_t        length;
> -       uint32_t        mode;
> -       uint32_t        padding;
> +       __u64   fh;
> +       __u64   offset;
> +       __u64   length;
> +       __u32   mode;
> +       __u32   padding;
>  };
>
>  struct fuse_in_header {
> -       uint32_t        len;
> -       uint32_t        opcode;
> -       uint64_t        unique;
> -       uint64_t        nodeid;
> -       uint32_t        uid;
> -       uint32_t        gid;
> -       uint32_t        pid;
> -       uint32_t        padding;
> +       __u32   len;
> +       __u32   opcode;
> +       __u64   unique;
> +       __u64   nodeid;
> +       __u32   uid;
> +       __u32   gid;
> +       __u32   pid;
> +       __u32   padding;
>  };
>
>  struct fuse_out_header {
> -       uint32_t        len;
> -       int32_t         error;
> -       uint64_t        unique;
> +       __u32   len;
> +       __s32           error;
> +       __u64   unique;
>  };
>
>  struct fuse_dirent {
> -       uint64_t        ino;
> -       uint64_t        off;
> -       uint32_t        namelen;
> -       uint32_t        type;
> +       __u64   ino;
> +       __u64   off;
> +       __u32   namelen;
> +       __u32   type;
>         char name[];
>  };
>
>  /* Align variable length records to 64bit boundary */
>  #define FUSE_REC_ALIGN(x) \
> -       (((x) + sizeof(uint64_t) - 1) & ~(sizeof(uint64_t) - 1))
> +       (((x) + sizeof(__u64) - 1) & ~(sizeof(__u64) - 1))
>
>  #define FUSE_NAME_OFFSET offsetof(struct fuse_dirent, name)
>  #define FUSE_DIRENT_ALIGN(x) FUSE_REC_ALIGN(x)
> @@ -907,106 +904,106 @@ struct fuse_direntplus {
>         FUSE_DIRENT_ALIGN(FUSE_NAME_OFFSET_DIRENTPLUS + (d)->dirent.namelen)
>
>  struct fuse_notify_inval_inode_out {
> -       uint64_t        ino;
> -       int64_t         off;
> -       int64_t         len;
> +       __u64   ino;
> +       __s64   off;
> +       __s64   len;
>  };
>
>  struct fuse_notify_inval_entry_out {
> -       uint64_t        parent;
> -       uint32_t        namelen;
> -       uint32_t        padding;
> +       __u64   parent;
> +       __u32   namelen;
> +       __u32   padding;
>  };
>
>  struct fuse_notify_delete_out {
> -       uint64_t        parent;
> -       uint64_t        child;
> -       uint32_t        namelen;
> -       uint32_t        padding;
> +       __u64   parent;
> +       __u64   child;
> +       __u32   namelen;
> +       __u32   padding;
>  };
>
>  struct fuse_notify_store_out {
> -       uint64_t        nodeid;
> -       uint64_t        offset;
> -       uint32_t        size;
> -       uint32_t        padding;
> +       __u64   nodeid;
> +       __u64   offset;
> +       __u32   size;
> +       __u32   padding;
>  };
>
>  struct fuse_notify_retrieve_out {
> -       uint64_t        notify_unique;
> -       uint64_t        nodeid;
> -       uint64_t        offset;
> -       uint32_t        size;
> -       uint32_t        padding;
> +       __u64   notify_unique;
> +       __u64   nodeid;
> +       __u64   offset;
> +       __u32   size;
> +       __u32   padding;
>  };
>
>  /* Matches the size of fuse_write_in */
>  struct fuse_notify_retrieve_in {
> -       uint64_t        dummy1;
> -       uint64_t        offset;
> -       uint32_t        size;
> -       uint32_t        dummy2;
> -       uint64_t        dummy3;
> -       uint64_t        dummy4;
> +       __u64   dummy1;
> +       __u64   offset;
> +       __u32   size;
> +       __u32   dummy2;
> +       __u64   dummy3;
> +       __u64   dummy4;
>  };
>
>  /* Device ioctls: */
>  #define FUSE_DEV_IOC_MAGIC             229
> -#define FUSE_DEV_IOC_CLONE             _IOR(FUSE_DEV_IOC_MAGIC, 0, uint32_t)
> +#define FUSE_DEV_IOC_CLONE             _IOR(FUSE_DEV_IOC_MAGIC, 0, __u32)
>
>  struct fuse_lseek_in {
> -       uint64_t        fh;
> -       uint64_t        offset;
> -       uint32_t        whence;
> -       uint32_t        padding;
> +       __u64   fh;
> +       __u64   offset;
> +       __u32   whence;
> +       __u32   padding;
>  };
>
>  struct fuse_lseek_out {
> -       uint64_t        offset;
> +       __u64   offset;
>  };
>
>  struct fuse_copy_file_range_in {
> -       uint64_t        fh_in;
> -       uint64_t        off_in;
> -       uint64_t        nodeid_out;
> -       uint64_t        fh_out;
> -       uint64_t        off_out;
> -       uint64_t        len;
> -       uint64_t        flags;
> +       __u64   fh_in;
> +       __u64   off_in;
> +       __u64   nodeid_out;
> +       __u64   fh_out;
> +       __u64   off_out;
> +       __u64   len;
> +       __u64   flags;
>  };
>
>  #define FUSE_SETUPMAPPING_FLAG_WRITE (1ull << 0)
>  #define FUSE_SETUPMAPPING_FLAG_READ (1ull << 1)
>  struct fuse_setupmapping_in {
>         /* An already open handle */
> -       uint64_t        fh;
> +       __u64   fh;
>         /* Offset into the file to start the mapping */
> -       uint64_t        foffset;
> +       __u64   foffset;
>         /* Length of mapping required */
> -       uint64_t        len;
> +       __u64   len;
>         /* Flags, FUSE_SETUPMAPPING_FLAG_* */
> -       uint64_t        flags;
> +       __u64   flags;
>         /* Offset in Memory Window */
> -       uint64_t        moffset;
> +       __u64   moffset;
>  };
>
>  struct fuse_removemapping_in {
>         /* number of fuse_removemapping_one follows */
> -       uint32_t        count;
> +       __u32   count;
>  };
>
>  struct fuse_removemapping_one {
>         /* Offset into the dax window start the unmapping */
> -       uint64_t        moffset;
> +       __u64   moffset;
>         /* Length of mapping required */
> -       uint64_t        len;
> +       __u64   len;
>  };
>
>  #define FUSE_REMOVEMAPPING_MAX_ENTRY   \
>                 (PAGE_SIZE / sizeof(struct fuse_removemapping_one))
>
>  struct fuse_syncfs_in {
> -       uint64_t        padding;
> +       __u64   padding;
>  };
>
>  /*
> @@ -1016,8 +1013,8 @@ struct fuse_syncfs_in {
>   * fuse_secctx, name, context
>   */
>  struct fuse_secctx {
> -       uint32_t        size;
> -       uint32_t        padding;
> +       __u32   size;
> +       __u32   padding;
>  };
>
>  /*
> @@ -1027,8 +1024,8 @@ struct fuse_secctx {
>   *
>   */
>  struct fuse_secctx_header {
> -       uint32_t        size;
> -       uint32_t        nr_secctx;
> +       __u32   size;
> +       __u32   nr_secctx;
>  };
>
>  #endif /* _LINUX_FUSE_H */
> --
> 2.35.1.894.gb6a874cedc-goog
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fuse: fix integer type usage in uapi header
  2022-03-18 17:14 [PATCH] fuse: fix integer type usage in uapi header Carlos Llamas
  2022-03-18 17:23 ` Masahiro Yamada
@ 2022-03-18 19:24 ` Miklos Szeredi
  2022-03-19  6:40   ` Greg Kroah-Hartman
  2022-03-21  2:07   ` Carlos Llamas
  2022-03-19  6:40 ` Greg Kroah-Hartman
  2 siblings, 2 replies; 12+ messages in thread
From: Miklos Szeredi @ 2022-03-18 19:24 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Greg Kroah-Hartman, Alessio Balsini, Masahiro Yamada,
	Arnd Bergmann, kernel-team, linux-fsdevel, linux-kernel

On Fri, 18 Mar 2022 at 18:14, Carlos Llamas <cmllamas@google.com> wrote:
>
> Kernel uapi headers are supposed to use __[us]{8,16,32,64} defined by
> <linux/types.h> instead of 'uint32_t' and similar. This patch changes
> all the definitions in this header to use the correct type. Previous
> discussion of this topic can be found here:
>
>   https://lkml.org/lkml/2019/6/5/18

This is effectively a revert of these two commits:

4c82456eeb4d ("fuse: fix type definitions in uapi header")
7e98d53086d1 ("Synchronize fuse header with one used in library")

And so we've gone full circle and back to having to modify the header
to be usable in the cross platform library...

And also made lots of churn for what reason exactly?

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fuse: fix integer type usage in uapi header
  2022-03-18 19:24 ` Miklos Szeredi
@ 2022-03-19  6:40   ` Greg Kroah-Hartman
  2022-03-21  2:07   ` Carlos Llamas
  1 sibling, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2022-03-19  6:40 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Carlos Llamas, Alessio Balsini, Masahiro Yamada, Arnd Bergmann,
	kernel-team, linux-fsdevel, linux-kernel

On Fri, Mar 18, 2022 at 08:24:55PM +0100, Miklos Szeredi wrote:
> On Fri, 18 Mar 2022 at 18:14, Carlos Llamas <cmllamas@google.com> wrote:
> >
> > Kernel uapi headers are supposed to use __[us]{8,16,32,64} defined by
> > <linux/types.h> instead of 'uint32_t' and similar. This patch changes
> > all the definitions in this header to use the correct type. Previous
> > discussion of this topic can be found here:
> >
> >   https://lkml.org/lkml/2019/6/5/18
> 
> This is effectively a revert of these two commits:
> 
> 4c82456eeb4d ("fuse: fix type definitions in uapi header")

That's a really odd commit, and should not have been recommended.  uapi
headers have to use __u32 and friends, otherwise things can be wrong.

> 7e98d53086d1 ("Synchronize fuse header with one used in library")
> 
> And so we've gone full circle and back to having to modify the header
> to be usable in the cross platform library...
> 
> And also made lots of churn for what reason exactly?

I think the original change above was wrong.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fuse: fix integer type usage in uapi header
  2022-03-18 17:14 [PATCH] fuse: fix integer type usage in uapi header Carlos Llamas
  2022-03-18 17:23 ` Masahiro Yamada
  2022-03-18 19:24 ` Miklos Szeredi
@ 2022-03-19  6:40 ` Greg Kroah-Hartman
  2 siblings, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2022-03-19  6:40 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Miklos Szeredi, Alessio Balsini, Masahiro Yamada, Arnd Bergmann,
	kernel-team, linux-fsdevel, linux-kernel

On Fri, Mar 18, 2022 at 05:14:05PM +0000, Carlos Llamas wrote:
> Kernel uapi headers are supposed to use __[us]{8,16,32,64} defined by
> <linux/types.h> instead of 'uint32_t' and similar. This patch changes
> all the definitions in this header to use the correct type. Previous
> discussion of this topic can be found here:
> 
>   https://lkml.org/lkml/2019/6/5/18
> 
> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> ---
>  include/uapi/linux/fuse.h | 509 +++++++++++++++++++-------------------
>  1 file changed, 253 insertions(+), 256 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fuse: fix integer type usage in uapi header
  2022-03-18 19:24 ` Miklos Szeredi
  2022-03-19  6:40   ` Greg Kroah-Hartman
@ 2022-03-21  2:07   ` Carlos Llamas
  2022-03-21  8:40     ` Miklos Szeredi
  1 sibling, 1 reply; 12+ messages in thread
From: Carlos Llamas @ 2022-03-21  2:07 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Greg Kroah-Hartman, Alessio Balsini, Masahiro Yamada,
	Arnd Bergmann, kernel-team, linux-fsdevel, linux-kernel

On Fri, Mar 18, 2022 at 08:24:55PM +0100, Miklos Szeredi wrote:
> On Fri, 18 Mar 2022 at 18:14, Carlos Llamas <cmllamas@google.com> wrote:
> >
> > Kernel uapi headers are supposed to use __[us]{8,16,32,64} defined by
> > <linux/types.h> instead of 'uint32_t' and similar. This patch changes
> > all the definitions in this header to use the correct type. Previous
> > discussion of this topic can be found here:
> >
> >   https://lkml.org/lkml/2019/6/5/18
> 
> This is effectively a revert of these two commits:
> 
> 4c82456eeb4d ("fuse: fix type definitions in uapi header")
> 7e98d53086d1 ("Synchronize fuse header with one used in library")
> 
> And so we've gone full circle and back to having to modify the header
> to be usable in the cross platform library...
> 
> And also made lots of churn for what reason exactly?

There are currently only two uapi headers making use of C99 types and
one is <linux/fuse.h>. This approach results in different typedefs being
selected when compiling for userspace vs the kernel. Plus only __u32 and
similar types align with the coding style as described in 5(e).

Yet, there is still the cross platform concern you mention. I think the
best way to accommodate this while still conforming with the __u32 types
is to follow something similar to 1a95916f5465 ("drm: Add compatibility
#ifdefs for *BSD"). Basically doing this:

  #if defined(__KERNEL__) || defined(__linux__)
  #include <linux/types.h>
  #else
  #include <stdint.h>
  typedef uint16_t __u16;
  typedef int32_t  __s32;
  typedef uint32_t __u32;
  typedef int64_t  __s64;
  typedef uint64_t __u64;
  #endif

This alternative selects the correct uapi types for both __KERNEL__ and
__linux__ cases which is the main goal of this patch and it's just minor
fixes from 7e98d53086d1 ("Synchronize header with one used in library").

I see there where previous attempts to address similar changes here:
  https://lkml.org/lkml/2013/3/11/620
  https://lkml.org/lkml/2013/4/15/487

So, if you agree with the approach above I'd be happy to send a separate
patch on top to address the *BSD compatibility.

Thanks,
Carlos Llamas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fuse: fix integer type usage in uapi header
  2022-03-21  2:07   ` Carlos Llamas
@ 2022-03-21  8:40     ` Miklos Szeredi
  2022-03-21  8:50       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 12+ messages in thread
From: Miklos Szeredi @ 2022-03-21  8:40 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Greg Kroah-Hartman, Alessio Balsini, Masahiro Yamada,
	Arnd Bergmann, kernel-team, linux-fsdevel, linux-kernel

On Mon, 21 Mar 2022 at 03:07, Carlos Llamas <cmllamas@google.com> wrote:
>
> On Fri, Mar 18, 2022 at 08:24:55PM +0100, Miklos Szeredi wrote:
> > On Fri, 18 Mar 2022 at 18:14, Carlos Llamas <cmllamas@google.com> wrote:
> > >
> > > Kernel uapi headers are supposed to use __[us]{8,16,32,64} defined by
> > > <linux/types.h> instead of 'uint32_t' and similar. This patch changes
> > > all the definitions in this header to use the correct type. Previous
> > > discussion of this topic can be found here:
> > >
> > >   https://lkml.org/lkml/2019/6/5/18
> >
> > This is effectively a revert of these two commits:
> >
> > 4c82456eeb4d ("fuse: fix type definitions in uapi header")
> > 7e98d53086d1 ("Synchronize fuse header with one used in library")
> >
> > And so we've gone full circle and back to having to modify the header
> > to be usable in the cross platform library...
> >
> > And also made lots of churn for what reason exactly?
>
> There are currently only two uapi headers making use of C99 types and
> one is <linux/fuse.h>. This approach results in different typedefs being
> selected when compiling for userspace vs the kernel.

Why is this a problem if the size of the resulting types is the same?

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fuse: fix integer type usage in uapi header
  2022-03-21  8:40     ` Miklos Szeredi
@ 2022-03-21  8:50       ` Greg Kroah-Hartman
  2022-03-21  9:36         ` Miklos Szeredi
  0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2022-03-21  8:50 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Carlos Llamas, Alessio Balsini, Masahiro Yamada, Arnd Bergmann,
	kernel-team, linux-fsdevel, linux-kernel

On Mon, Mar 21, 2022 at 09:40:56AM +0100, Miklos Szeredi wrote:
> On Mon, 21 Mar 2022 at 03:07, Carlos Llamas <cmllamas@google.com> wrote:
> >
> > On Fri, Mar 18, 2022 at 08:24:55PM +0100, Miklos Szeredi wrote:
> > > On Fri, 18 Mar 2022 at 18:14, Carlos Llamas <cmllamas@google.com> wrote:
> > > >
> > > > Kernel uapi headers are supposed to use __[us]{8,16,32,64} defined by
> > > > <linux/types.h> instead of 'uint32_t' and similar. This patch changes
> > > > all the definitions in this header to use the correct type. Previous
> > > > discussion of this topic can be found here:
> > > >
> > > >   https://lkml.org/lkml/2019/6/5/18
> > >
> > > This is effectively a revert of these two commits:
> > >
> > > 4c82456eeb4d ("fuse: fix type definitions in uapi header")
> > > 7e98d53086d1 ("Synchronize fuse header with one used in library")
> > >
> > > And so we've gone full circle and back to having to modify the header
> > > to be usable in the cross platform library...
> > >
> > > And also made lots of churn for what reason exactly?
> >
> > There are currently only two uapi headers making use of C99 types and
> > one is <linux/fuse.h>. This approach results in different typedefs being
> > selected when compiling for userspace vs the kernel.
> 
> Why is this a problem if the size of the resulting types is the same?

uint* are not "valid" variable types to cross the user/kernel boundary.
They are part of the userspace variable type namespace, not the kernel
variable type namespace.  Linus wrong a long post about this somewhere
in the past, I'm sure someone can dig it up...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fuse: fix integer type usage in uapi header
  2022-03-21  8:50       ` Greg Kroah-Hartman
@ 2022-03-21  9:36         ` Miklos Szeredi
  2022-03-21 10:01           ` Greg Kroah-Hartman
  0 siblings, 1 reply; 12+ messages in thread
From: Miklos Szeredi @ 2022-03-21  9:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Carlos Llamas, Alessio Balsini, Masahiro Yamada, Arnd Bergmann,
	kernel-team, linux-fsdevel, linux-kernel

On Mon, 21 Mar 2022 at 09:50, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Mar 21, 2022 at 09:40:56AM +0100, Miklos Szeredi wrote:
> > On Mon, 21 Mar 2022 at 03:07, Carlos Llamas <cmllamas@google.com> wrote:
> > >
> > > On Fri, Mar 18, 2022 at 08:24:55PM +0100, Miklos Szeredi wrote:
> > > > On Fri, 18 Mar 2022 at 18:14, Carlos Llamas <cmllamas@google.com> wrote:
> > > > >
> > > > > Kernel uapi headers are supposed to use __[us]{8,16,32,64} defined by
> > > > > <linux/types.h> instead of 'uint32_t' and similar. This patch changes
> > > > > all the definitions in this header to use the correct type. Previous
> > > > > discussion of this topic can be found here:
> > > > >
> > > > >   https://lkml.org/lkml/2019/6/5/18
> > > >
> > > > This is effectively a revert of these two commits:
> > > >
> > > > 4c82456eeb4d ("fuse: fix type definitions in uapi header")
> > > > 7e98d53086d1 ("Synchronize fuse header with one used in library")
> > > >
> > > > And so we've gone full circle and back to having to modify the header
> > > > to be usable in the cross platform library...
> > > >
> > > > And also made lots of churn for what reason exactly?
> > >
> > > There are currently only two uapi headers making use of C99 types and
> > > one is <linux/fuse.h>. This approach results in different typedefs being
> > > selected when compiling for userspace vs the kernel.
> >
> > Why is this a problem if the size of the resulting types is the same?
>
> uint* are not "valid" variable types to cross the user/kernel boundary.
> They are part of the userspace variable type namespace, not the kernel
> variable type namespace.  Linus wrong a long post about this somewhere
> in the past, I'm sure someone can dig it up...

Looking forward to the details.  I cannot imagine why this would matter...

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fuse: fix integer type usage in uapi header
  2022-03-21  9:36         ` Miklos Szeredi
@ 2022-03-21 10:01           ` Greg Kroah-Hartman
  2022-03-21 11:25             ` Miklos Szeredi
  0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2022-03-21 10:01 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Carlos Llamas, Alessio Balsini, Masahiro Yamada, Arnd Bergmann,
	kernel-team, linux-fsdevel, linux-kernel

On Mon, Mar 21, 2022 at 10:36:20AM +0100, Miklos Szeredi wrote:
> On Mon, 21 Mar 2022 at 09:50, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Mar 21, 2022 at 09:40:56AM +0100, Miklos Szeredi wrote:
> > > On Mon, 21 Mar 2022 at 03:07, Carlos Llamas <cmllamas@google.com> wrote:
> > > >
> > > > On Fri, Mar 18, 2022 at 08:24:55PM +0100, Miklos Szeredi wrote:
> > > > > On Fri, 18 Mar 2022 at 18:14, Carlos Llamas <cmllamas@google.com> wrote:
> > > > > >
> > > > > > Kernel uapi headers are supposed to use __[us]{8,16,32,64} defined by
> > > > > > <linux/types.h> instead of 'uint32_t' and similar. This patch changes
> > > > > > all the definitions in this header to use the correct type. Previous
> > > > > > discussion of this topic can be found here:
> > > > > >
> > > > > >   https://lkml.org/lkml/2019/6/5/18
> > > > >
> > > > > This is effectively a revert of these two commits:
> > > > >
> > > > > 4c82456eeb4d ("fuse: fix type definitions in uapi header")
> > > > > 7e98d53086d1 ("Synchronize fuse header with one used in library")
> > > > >
> > > > > And so we've gone full circle and back to having to modify the header
> > > > > to be usable in the cross platform library...
> > > > >
> > > > > And also made lots of churn for what reason exactly?
> > > >
> > > > There are currently only two uapi headers making use of C99 types and
> > > > one is <linux/fuse.h>. This approach results in different typedefs being
> > > > selected when compiling for userspace vs the kernel.
> > >
> > > Why is this a problem if the size of the resulting types is the same?
> >
> > uint* are not "valid" variable types to cross the user/kernel boundary.
> > They are part of the userspace variable type namespace, not the kernel
> > variable type namespace.  Linus wrong a long post about this somewhere
> > in the past, I'm sure someone can dig it up...
> 
> Looking forward to the details.  I cannot imagine why this would matter...

Here's the huge thread on the issue:
	https://lore.kernel.org/all/19865.1101395592@redhat.com/
and specifically here's Linus's answer:
	https://lore.kernel.org/all/Pine.LNX.4.58.0411281710490.22796@ppc970.osdl.org/

The whole thread is actually relevant for this .h file as well.  Some
things never change :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fuse: fix integer type usage in uapi header
  2022-03-21 10:01           ` Greg Kroah-Hartman
@ 2022-03-21 11:25             ` Miklos Szeredi
  2022-03-21 12:28               ` Greg Kroah-Hartman
  0 siblings, 1 reply; 12+ messages in thread
From: Miklos Szeredi @ 2022-03-21 11:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Carlos Llamas, Alessio Balsini, Masahiro Yamada, Arnd Bergmann,
	kernel-team, linux-fsdevel, linux-kernel

On Mon, 21 Mar 2022 at 11:01, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Mar 21, 2022 at 10:36:20AM +0100, Miklos Szeredi wrote:
> > On Mon, 21 Mar 2022 at 09:50, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Mon, Mar 21, 2022 at 09:40:56AM +0100, Miklos Szeredi wrote:
> > > > On Mon, 21 Mar 2022 at 03:07, Carlos Llamas <cmllamas@google.com> wrote:
> > > > >
> > > > > On Fri, Mar 18, 2022 at 08:24:55PM +0100, Miklos Szeredi wrote:
> > > > > > On Fri, 18 Mar 2022 at 18:14, Carlos Llamas <cmllamas@google.com> wrote:
> > > > > > >
> > > > > > > Kernel uapi headers are supposed to use __[us]{8,16,32,64} defined by
> > > > > > > <linux/types.h> instead of 'uint32_t' and similar. This patch changes
> > > > > > > all the definitions in this header to use the correct type. Previous
> > > > > > > discussion of this topic can be found here:
> > > > > > >
> > > > > > >   https://lkml.org/lkml/2019/6/5/18
> > > > > >
> > > > > > This is effectively a revert of these two commits:
> > > > > >
> > > > > > 4c82456eeb4d ("fuse: fix type definitions in uapi header")
> > > > > > 7e98d53086d1 ("Synchronize fuse header with one used in library")
> > > > > >
> > > > > > And so we've gone full circle and back to having to modify the header
> > > > > > to be usable in the cross platform library...
> > > > > >
> > > > > > And also made lots of churn for what reason exactly?
> > > > >
> > > > > There are currently only two uapi headers making use of C99 types and
> > > > > one is <linux/fuse.h>. This approach results in different typedefs being
> > > > > selected when compiling for userspace vs the kernel.
> > > >
> > > > Why is this a problem if the size of the resulting types is the same?
> > >
> > > uint* are not "valid" variable types to cross the user/kernel boundary.
> > > They are part of the userspace variable type namespace, not the kernel
> > > variable type namespace.  Linus wrong a long post about this somewhere
> > > in the past, I'm sure someone can dig it up...
> >
> > Looking forward to the details.  I cannot imagine why this would matter...
>
> Here's the huge thread on the issue:
>         https://lore.kernel.org/all/19865.1101395592@redhat.com/
> and specifically here's Linus's answer:
>         https://lore.kernel.org/all/Pine.LNX.4.58.0411281710490.22796@ppc970.osdl.org/
>
> The whole thread is actually relevant for this .h file as well.  Some
> things never change :)

"- the kernel should not depend on, or pollute user-space naming.
  YOU MUST NOT USE "uint32_t" when that may not be defined, and
  user-space rules for when it is defined are arcane and totally
  arbitrary."

The "pollutes user space naming" argument is bogus for fuse, since
application are using the library interface, which doesn't pull in the
kernel headers but redefines everything that needs to be shared.   BTW
this seems to be the pattern for libc interfaces as well, though I
haven't looked closely.

On the other hand, if we change the types back to __u32 etc, then that
will mess with the history.  I think the disadvantages outweigh the
advantages, so unless some stronger argument comes up  it's NACK from
me.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fuse: fix integer type usage in uapi header
  2022-03-21 11:25             ` Miklos Szeredi
@ 2022-03-21 12:28               ` Greg Kroah-Hartman
  0 siblings, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2022-03-21 12:28 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Carlos Llamas, Alessio Balsini, Masahiro Yamada, Arnd Bergmann,
	kernel-team, linux-fsdevel, linux-kernel

On Mon, Mar 21, 2022 at 12:25:27PM +0100, Miklos Szeredi wrote:
> On Mon, 21 Mar 2022 at 11:01, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Mar 21, 2022 at 10:36:20AM +0100, Miklos Szeredi wrote:
> > > On Mon, 21 Mar 2022 at 09:50, Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Mon, Mar 21, 2022 at 09:40:56AM +0100, Miklos Szeredi wrote:
> > > > > On Mon, 21 Mar 2022 at 03:07, Carlos Llamas <cmllamas@google.com> wrote:
> > > > > >
> > > > > > On Fri, Mar 18, 2022 at 08:24:55PM +0100, Miklos Szeredi wrote:
> > > > > > > On Fri, 18 Mar 2022 at 18:14, Carlos Llamas <cmllamas@google.com> wrote:
> > > > > > > >
> > > > > > > > Kernel uapi headers are supposed to use __[us]{8,16,32,64} defined by
> > > > > > > > <linux/types.h> instead of 'uint32_t' and similar. This patch changes
> > > > > > > > all the definitions in this header to use the correct type. Previous
> > > > > > > > discussion of this topic can be found here:
> > > > > > > >
> > > > > > > >   https://lkml.org/lkml/2019/6/5/18
> > > > > > >
> > > > > > > This is effectively a revert of these two commits:
> > > > > > >
> > > > > > > 4c82456eeb4d ("fuse: fix type definitions in uapi header")
> > > > > > > 7e98d53086d1 ("Synchronize fuse header with one used in library")
> > > > > > >
> > > > > > > And so we've gone full circle and back to having to modify the header
> > > > > > > to be usable in the cross platform library...
> > > > > > >
> > > > > > > And also made lots of churn for what reason exactly?
> > > > > >
> > > > > > There are currently only two uapi headers making use of C99 types and
> > > > > > one is <linux/fuse.h>. This approach results in different typedefs being
> > > > > > selected when compiling for userspace vs the kernel.
> > > > >
> > > > > Why is this a problem if the size of the resulting types is the same?
> > > >
> > > > uint* are not "valid" variable types to cross the user/kernel boundary.
> > > > They are part of the userspace variable type namespace, not the kernel
> > > > variable type namespace.  Linus wrong a long post about this somewhere
> > > > in the past, I'm sure someone can dig it up...
> > >
> > > Looking forward to the details.  I cannot imagine why this would matter...
> >
> > Here's the huge thread on the issue:
> >         https://lore.kernel.org/all/19865.1101395592@redhat.com/
> > and specifically here's Linus's answer:
> >         https://lore.kernel.org/all/Pine.LNX.4.58.0411281710490.22796@ppc970.osdl.org/
> >
> > The whole thread is actually relevant for this .h file as well.  Some
> > things never change :)
> 
> "- the kernel should not depend on, or pollute user-space naming.
>   YOU MUST NOT USE "uint32_t" when that may not be defined, and
>   user-space rules for when it is defined are arcane and totally
>   arbitrary."
> 
> The "pollutes user space naming" argument is bogus for fuse, since
> application are using the library interface, which doesn't pull in the
> kernel headers but redefines everything that needs to be shared.   BTW
> this seems to be the pattern for libc interfaces as well, though I
> haven't looked closely.
> 
> On the other hand, if we change the types back to __u32 etc, then that
> will mess with the history.  I think the disadvantages outweigh the
> advantages, so unless some stronger argument comes up  it's NACK from
> me.

As this .h file is only 1 of 3 .h files using these variable types, I
think you are wrong and should go along with the rest of the kernel api
style.

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-03-21 12:28 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-18 17:14 [PATCH] fuse: fix integer type usage in uapi header Carlos Llamas
2022-03-18 17:23 ` Masahiro Yamada
2022-03-18 19:24 ` Miklos Szeredi
2022-03-19  6:40   ` Greg Kroah-Hartman
2022-03-21  2:07   ` Carlos Llamas
2022-03-21  8:40     ` Miklos Szeredi
2022-03-21  8:50       ` Greg Kroah-Hartman
2022-03-21  9:36         ` Miklos Szeredi
2022-03-21 10:01           ` Greg Kroah-Hartman
2022-03-21 11:25             ` Miklos Szeredi
2022-03-21 12:28               ` Greg Kroah-Hartman
2022-03-19  6:40 ` Greg Kroah-Hartman

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.