UAPI/ioctl: Improve parameter name of ioctl request definition helpers

[PATCH] UAPI/ioctl: Improve parameter name of ioctl request definition helpers

Posted by Uwe Kleine-König 1 year, 7 months ago

The third parameter to _IOR et al is a type name, not a size. So the
parameter being named "size" is irritating. Rename it to "argtype"
instead to reduce confusion.

There is a very minor chance that this breaks stuff. It only hurts
however if there is a variable (or macro) in userspace that is called
"argtype" *and* it's used in the parameters of _IOR and friends. IMHO
this is negligible because usually definitions making use of these
macros are provided by kernel headers (i.e. us) or if they are
replicated in userspace code, they are replicated and so supposed to
match the kernel definitions (e.g. to make them usable by programs
without the need to update the kernel headers used to compile the
program).

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
Hello,

if there are doubts about using "argtype": Would "_argtype" be better?

Best regards
Uwe

 include/uapi/asm-generic/ioctl.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/uapi/asm-generic/ioctl.h b/include/uapi/asm-generic/ioctl.h
index a84f4db8a250..e3290a5824c9 100644
--- a/include/uapi/asm-generic/ioctl.h
+++ b/include/uapi/asm-generic/ioctl.h
@@ -82,13 +82,13 @@
  * NOTE: _IOW means userland is writing and kernel is reading. _IOR
  * means userland is reading and kernel is writing.
  */
-#define _IO(type,nr)		_IOC(_IOC_NONE,(type),(nr),0)
-#define _IOR(type,nr,size)	_IOC(_IOC_READ,(type),(nr),(_IOC_TYPECHECK(size)))
-#define _IOW(type,nr,size)	_IOC(_IOC_WRITE,(type),(nr),(_IOC_TYPECHECK(size)))
-#define _IOWR(type,nr,size)	_IOC(_IOC_READ|_IOC_WRITE,(type),(nr),(_IOC_TYPECHECK(size)))
-#define _IOR_BAD(type,nr,size)	_IOC(_IOC_READ,(type),(nr),sizeof(size))
-#define _IOW_BAD(type,nr,size)	_IOC(_IOC_WRITE,(type),(nr),sizeof(size))
-#define _IOWR_BAD(type,nr,size)	_IOC(_IOC_READ|_IOC_WRITE,(type),(nr),sizeof(size))
+#define _IO(type,nr)			_IOC(_IOC_NONE,(type),(nr),0)
+#define _IOR(type,nr,argtype)		_IOC(_IOC_READ,(type),(nr),(_IOC_TYPECHECK(argtype)))
+#define _IOW(type,nr,argtype)		_IOC(_IOC_WRITE,(type),(nr),(_IOC_TYPECHECK(argtype)))
+#define _IOWR(type,nr,argtype)		_IOC(_IOC_READ|_IOC_WRITE,(type),(nr),(_IOC_TYPECHECK(argtype)))
+#define _IOR_BAD(type,nr,argtype)	_IOC(_IOC_READ,(type),(nr),sizeof(argtype))
+#define _IOW_BAD(type,nr,argtype)	_IOC(_IOC_WRITE,(type),(nr),sizeof(argtype))
+#define _IOWR_BAD(type,nr,argtype)	_IOC(_IOC_READ|_IOC_WRITE,(type),(nr),sizeof(argtype))
 
 /* used to decode ioctl numbers.. */
 #define _IOC_DIR(nr)		(((nr) >> _IOC_DIRSHIFT) & _IOC_DIRMASK)

base-commit: 3fe121b622825ff8cc995a1e6b026181c48188db
prerequisite-patch-id: 816efa50518af0814a168f7f0ac5904b7128e5b1
-- 
2.43.0

Re: [PATCH] UAPI/ioctl: Improve parameter name of ioctl request definition helpers

Posted by Arnd Bergmann 1 year, 7 months ago

On Fri, Jul 12, 2024, at 11:35, Uwe Kleine-König wrote:
> The third parameter to _IOR et al is a type name, not a size. So the
> parameter being named "size" is irritating. Rename it to "argtype"
> instead to reduce confusion.
>
> There is a very minor chance that this breaks stuff. It only hurts
> however if there is a variable (or macro) in userspace that is called
> "argtype" *and* it's used in the parameters of _IOR and friends. IMHO
> this is negligible because usually definitions making use of these
> macros are provided by kernel headers (i.e. us) or if they are
> replicated in userspace code, they are replicated and so supposed to
> match the kernel definitions (e.g. to make them usable by programs
> without the need to update the kernel headers used to compile the
> program).
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> Hello,
>
> if there are doubts about using "argtype": Would "_argtype" be better?

The patch looks good to me, and I think using 'argtype'
is fine. I would apply it directly, but not with the current
timing just ahead of the merge window.

If there are no other comments, how about I take this after -rc1?
You may have to remind me about it.

      Arnd

Re: [PATCH] UAPI/ioctl: Improve parameter name of ioctl request definition helpers

Posted by Uwe Kleine-König 1 year, 7 months ago

Hey Arnd,

On Fri, Jul 12, 2024 at 11:51:38AM +0200, Arnd Bergmann wrote:
> On Fri, Jul 12, 2024, at 11:35, Uwe Kleine-König wrote:
> > if there are doubts about using "argtype": Would "_argtype" be better?
> 
> The patch looks good to me, and I think using 'argtype'
> is fine. I would apply it directly, but not with the current
> timing just ahead of the merge window.

Yes, having it in next for some time is a good idea for sure.

> If there are no other comments, how about I take this after -rc1?
> You may have to remind me about it.

I'll ping you.

Best regards
Uwe