From: Al Viro <viro@zeniv.linux.org.uk>
To: Jiri Slaby <jirislaby@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Alexey Gladkov <legion@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
kbd@lists.linux.dev, linux-api@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-serial@vger.kernel.org,
Arnd Bergmann <arnd@kernel.org>
Subject: Re: [PATCH v5 1/3] VT: Use macros to define ioctls
Date: Wed, 29 May 2024 09:44:43 +0100 [thread overview]
Message-ID: <20240529084443.GO2118490@ZenIV> (raw)
In-Reply-To: <0da9785e-ba44-4718-9d08-4e96c1ba7ab2@kernel.org>
On Wed, May 29, 2024 at 09:29:23AM +0200, Jiri Slaby wrote:
> On 18. 04. 24, 8:18, Greg Kroah-Hartman wrote:
> > On Wed, Apr 17, 2024 at 07:37:35PM +0200, Alexey Gladkov wrote:
> > > All other headers use _IOC() macros to describe ioctls for a long time
> > > now. This header is stuck in the last century.
> > >
> > > Simply use the _IO() macro. No other changes.
> > >
> > > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> > > ---
> > > include/uapi/linux/kd.h | 96 +++++++++++++++++++++--------------------
> > > 1 file changed, 49 insertions(+), 47 deletions(-)
> >
> > This is a nice cleanup, thanks for doing it, I'll just take this one
> > change now if you don't object.
>
> Unfortunately, _IOC_NONE is 1 on some archs as noted by Arnd, and this
> commit changed the kd ioctl values in there which broke stuff as noted by
> Al.
>
> We either:
> * use _IOC(0, X, Y) in here, instead of _IO(X, Y), or
> * define KDIOC(X) as _IOC(0, KD_IOCTL_BASE, X), or
> * revert the commit which landed to -rc1 already.
Revert, IMO. If nothing else, _IO... macros are there to
* document the copyin/copyout direction
* make sure that argument size mismatches between the kernel
and userland get caught
Turning 16bit hex constants into that won't serve any purpose other
than preventing such patches in the future; adding a comment would
deal with that just fine.
BTW, the original odd choice for _IOC_NONE appears to have been motivated
by avoiding conflicts with legacy ioctl numbers: in CSRG repo there's
this:
commit 02cbc6a653b48bb4ec6052cbe6b3979530011c46
Author: Sam Leffler <sam@ucbvax.Berkeley.EDU>
Date: Mon Aug 2 02:22:17 1982 -0800
new ioctl's
...
+/*
+ * Ioctl's have the command encoded in the lower word,
+ * and the size of any in or out parameters in the upper
+ * word. The high 2 bits of the upper word are used
+ * to encode the in/out status of the parameter; for now
+ * we restrict parameters to at most 128 bytes.
+ */
+#define IOCPARM_MASK 0x7f /* parameters must be < 128 bytes */
+#define IOC_VOID 0x20000000 /* no parameters */
+#define IOC_OUT 0x40000000 /* copy out parameters */
+#define IOC_IN 0x80000000 /* copy in parameters */
+#define IOC_INOUT (IOC_IN|IOC_OUT)
+/* the 0x20000000 is so we can distinguish new ioctl's from old */
+#define _IO(x,y) (IOC_VOID|('x'<<8)|y)
+#define _IOR(x,y,t) (IOC_OUT|((sizeof(t)&IOCPARM_MASK)<<16)|('x'<<8)|y)
+#define _IOW(x,y,t) (IOC_IN|((sizeof(t)&IOCPARM_MASK)<<16)|('x'<<8)|y)
+/* this should be _IORW, but stdio got there first */
+#define _IOWR(x,y,t) (IOC_INOUT|((sizeof(t)&IOCPARM_MASK)<<16)|('x'<<8)|y)
FWIW, the upper limit on parameter size went up from 128 bytes to 8Kb in
commit 856ed9ecd2795280feed42baf3889ac7c7f9a26d
Author: Mike Karels <karels@ucbvax.Berkeley.EDU>
Date: Thu Feb 19 22:16:28 1987 -0800
larger ioctl's (for disklabels)
Wonder what got us (in sparc port?) to lift it from 8Kb to 16Kb...
In our historical tree that has happened in
commit 1c29224f169362b671a4bef4cd864464ef810d42
Author: Pete Zaitcev <zaitcev@redhat.com>
Date: Sun Mar 2 08:45:12 2003 -0800
[SPARC32/64]: Expand ioctl size field in backwards-compatible way.
No explanations there...
next prev parent reply other threads:[~2024-05-29 8:44 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1708960303.git.legion@kernel.org>
2024-03-12 14:23 ` [PATCH v3 0/2] VT: Add ability to get font requirements legion
2024-03-12 14:23 ` [PATCH v3 1/2] VT: Add KDFONTINFO ioctl legion
2024-03-15 9:15 ` Helge Deller
2024-03-12 14:23 ` [PATCH v3 2/2] VT: Allow to get max font width and height legion
2024-03-13 17:40 ` Oleg Bulatov
2024-04-02 11:09 ` Jiri Slaby
2024-03-15 9:16 ` Helge Deller
2024-04-02 10:32 ` [RESEND PATCH v3 0/2] VT: Add ability to get font requirements Alexey Gladkov
2024-04-02 10:32 ` [RESEND PATCH v3 1/2] VT: Add KDFONTINFO ioctl Alexey Gladkov
2024-04-02 11:02 ` Jiri Slaby
2024-04-02 13:19 ` Alexey Gladkov
2024-04-03 5:27 ` Jiri Slaby
2024-04-10 16:29 ` Alexey Gladkov
2024-04-10 17:11 ` Greg Kroah-Hartman
2024-04-02 17:50 ` [PATCH v4 0/3] VT: Add ability to get font requirements Alexey Gladkov
2024-04-02 17:50 ` [PATCH v4 1/3] VT: Use macros to define ioctls Alexey Gladkov
2024-04-02 17:50 ` [PATCH v4 2/3] VT: Add KDFONTINFO ioctl Alexey Gladkov
2024-04-03 4:55 ` Greg Kroah-Hartman
2024-04-03 5:05 ` Jiri Slaby
2024-04-10 16:36 ` Alexey Gladkov
2024-04-11 3:53 ` Jiri Slaby
2024-04-02 17:50 ` [PATCH v4 3/3] VT: Allow to get max font width and height Alexey Gladkov
2024-04-17 17:37 ` [PATCH v5 0/3] VT: Add ability to get font requirements Alexey Gladkov
2024-04-17 17:37 ` [PATCH v5 1/3] VT: Use macros to define ioctls Alexey Gladkov
2024-04-18 6:18 ` Greg Kroah-Hartman
2024-05-29 7:29 ` Jiri Slaby
2024-05-29 7:44 ` Arnd Bergmann
2024-05-29 8:44 ` Al Viro [this message]
2024-04-17 17:37 ` [PATCH v5 2/3] VT: Add KDFONTINFO ioctl Alexey Gladkov
2024-04-17 19:31 ` Helge Deller
2024-04-18 10:45 ` Alexey Gladkov
2024-04-25 10:33 ` Helge Deller
2024-04-25 11:06 ` Alexey Gladkov
2024-04-25 11:35 ` Helge Deller
2024-04-18 6:18 ` Greg Kroah-Hartman
2024-04-18 10:27 ` Alexey Gladkov
2024-04-17 17:37 ` [PATCH v5 3/3] VT: Allow to get max font width and height Alexey Gladkov
2024-04-02 10:32 ` [RESEND PATCH v3 2/2] " Alexey Gladkov
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=20240529084443.GO2118490@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=arnd@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@kernel.org \
--cc=kbd@lists.linux.dev \
--cc=legion@kernel.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
/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 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 read-only IMAP folder(s) and NNTP newsgroup(s).