* [PATCH] UAPI linux/fs.h: add a warning about wrong ioctl argument types
@ 2013-11-27 10:40 Aurelien Jarno
2013-11-27 11:13 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Aurelien Jarno @ 2013-11-27 10:40 UTC (permalink / raw)
To: linux-kernel
Cc: Aurelien Jarno, Alexander Viro, Christoph Hellwig,
Theodore Ts'o, linux-fsdevel
The FS_IOC_GETFLAGS, FS_IOC_SETFLAGS, FS_IOC_GETVERSION and
FS_IOC_SETVERSION ioctls are declared for historical reasons with a long
argument, while in practice they take an int argument. This is
unfortunately not possible to fix that without breaking the kernel ABI.
However good programmers actually lookup for the argument type of these
ioctls in <fs/linux.h> and end-up using a long. This breaks on 64-bit
big endian machines.
Instead of letting the same "mistake" to be done again and again, add an
explanation to <fs/linux.h> so that people use the correct argument type
in their code. Also add a small comment on the same line while staying
within 80 characters so that the issue appears when doing a grep on this
file.
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Theodore Ts'o <tytso@mit.edu>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
include/uapi/linux/fs.h | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 6c28b61..baeb803 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -154,10 +154,16 @@ struct inodes_stat_t {
#define FITHAW _IOWR('X', 120, int) /* Thaw */
#define FITRIM _IOWR('X', 121, struct fstrim_range) /* Trim */
-#define FS_IOC_GETFLAGS _IOR('f', 1, long)
-#define FS_IOC_SETFLAGS _IOW('f', 2, long)
-#define FS_IOC_GETVERSION _IOR('v', 1, long)
-#define FS_IOC_SETVERSION _IOW('v', 2, long)
+/*
+ * WARNING: The next four following ioctls actually take an int argument
+ * despite their definition. This is important to support 64-bit big-endian
+ * machines.
+ */
+#define FS_IOC_GETFLAGS _IOR('f', 1, long) /* should be int */
+#define FS_IOC_SETFLAGS _IOW('f', 2, long) /* should be int */
+#define FS_IOC_GETVERSION _IOR('v', 1, long) /* should be int */
+#define FS_IOC_SETVERSION _IOW('v', 2, long) /* should be int */
+
#define FS_IOC_FIEMAP _IOWR('f', 11, struct fiemap)
#define FS_IOC32_GETFLAGS _IOR('f', 1, int)
#define FS_IOC32_SETFLAGS _IOW('f', 2, int)
--
1.8.4.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] UAPI linux/fs.h: add a warning about wrong ioctl argument types
2013-11-27 10:40 [PATCH] UAPI linux/fs.h: add a warning about wrong ioctl argument types Aurelien Jarno
@ 2013-11-27 11:13 ` Christoph Hellwig
2013-11-27 23:02 ` Aurelien Jarno
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2013-11-27 11:13 UTC (permalink / raw)
To: Aurelien Jarno
Cc: linux-kernel, Alexander Viro, Christoph Hellwig,
Theodore Ts'o, linux-fsdevel
Thanks Aurelien,
the added comment looks good and very useful,
Reviewed-by: Christoph Hellwig <hch@lst.de>
We don't have any man pages documenting these ioctls or other user
visible documentation, do we?
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] UAPI linux/fs.h: add a warning about wrong ioctl argument types
2013-11-27 11:13 ` Christoph Hellwig
@ 2013-11-27 23:02 ` Aurelien Jarno
0 siblings, 0 replies; 3+ messages in thread
From: Aurelien Jarno @ 2013-11-27 23:02 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-kernel, Alexander Viro, Theodore Ts'o, linux-fsdevel
On Wed, Nov 27, 2013 at 03:13:40AM -0800, Christoph Hellwig wrote:
> Thanks Aurelien,
>
> the added comment looks good and very useful,
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
Thanks for the review.
> We don't have any man pages documenting these ioctls or other user
> visible documentation, do we?
The only documentation I have found about these ioctls is from the
ioctl_list(2) manpage, but with different name and defined in
<linux/ext2_fs.h>. In any case they are correctly defined:
0x80046601 EXT2_IOC_GETFLAGS int *
0x40046602 EXT2_IOC_SETFLAGS const int *
0x80047601 EXT2_IOC_GETVERSION int *
0x40047602 EXT2_IOC_SETVERSION const int *
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-11-27 23:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-27 10:40 [PATCH] UAPI linux/fs.h: add a warning about wrong ioctl argument types Aurelien Jarno
2013-11-27 11:13 ` Christoph Hellwig
2013-11-27 23:02 ` Aurelien Jarno
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).