[PATCH] i2c-dev: Fix typo in ioctl name reference

[PATCH] i2c-dev: Fix typo in ioctl name reference

[Jean Delvare]

The ioctl is named I2C_RDWR for "I2C read/write". But references to it
were misspelled "rdrw". Fix them.

Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
---
 drivers/i2c/i2c-dev.c        |    6 +++---
 fs/compat_ioctl.c            |    2 +-
 include/uapi/linux/i2c-dev.h |    4 +++-
 3 files changed, 7 insertions(+), 5 deletions(-)

--- linux-4.2.orig/drivers/i2c/i2c-dev.c	2015-09-08 09:40:43.287812694 +0200
+++ linux-4.2/drivers/i2c/i2c-dev.c	2015-09-08 09:48:55.191927122 +0200
@@ -235,7 +235,7 @@ static int i2cdev_check_addr(struct i2c_
 	return result;
 }
 
-static noinline int i2cdev_ioctl_rdrw(struct i2c_client *client,
+static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
 		unsigned long arg)
 {
 	struct i2c_rdwr_ioctl_data rdwr_arg;
@@ -250,7 +250,7 @@ static noinline int i2cdev_ioctl_rdrw(st
 
 	/* Put an arbitrary limit on the number of messages that can
 	 * be sent at once */
-	if (rdwr_arg.nmsgs > I2C_RDRW_IOCTL_MAX_MSGS)
+	if (rdwr_arg.nmsgs > I2C_RDWR_IOCTL_MAX_MSGS)
 		return -EINVAL;
 
 	rdwr_pa = memdup_user(rdwr_arg.msgs,
@@ -456,7 +456,7 @@ static long i2cdev_ioctl(struct file *fi
 		return put_user(funcs, (unsigned long __user *)arg);
 
 	case I2C_RDWR:
-		return i2cdev_ioctl_rdrw(client, arg);
+		return i2cdev_ioctl_rdwr(client, arg);
 
 	case I2C_SMBUS:
 		return i2cdev_ioctl_smbus(client, arg);
--- linux-4.2.orig/fs/compat_ioctl.c	2015-09-08 09:40:43.287812694 +0200
+++ linux-4.2/fs/compat_ioctl.c	2015-09-08 09:48:55.193927164 +0200
@@ -686,7 +686,7 @@ static int do_i2c_rdwr_ioctl(unsigned in
 
 	if (get_user(nmsgs, &udata->nmsgs))
 		return -EFAULT;
-	if (nmsgs > I2C_RDRW_IOCTL_MAX_MSGS)
+	if (nmsgs > I2C_RDWR_IOCTL_MAX_MSGS)
 		return -EINVAL;
 
 	if (get_user(datap, &udata->msgs))
--- linux-4.2.orig/include/uapi/linux/i2c-dev.h	2015-09-08 09:40:43.287812694 +0200
+++ linux-4.2/include/uapi/linux/i2c-dev.h	2015-09-08 09:48:55.193927164 +0200
@@ -66,7 +66,9 @@ struct i2c_rdwr_ioctl_data {
 	__u32 nmsgs;			/* number of i2c_msgs */
 };
 
-#define  I2C_RDRW_IOCTL_MAX_MSGS	42
+#define  I2C_RDWR_IOCTL_MAX_MSGS	42
+/* Originally defined with a typo, keep if for now for compatibility */
+#define  I2C_RDRW_IOCTL_MAX_MSGS	I2C_RDWR_IOCTL_MAX_MSGS
 
 
 #endif /* _UAPI_LINUX_I2C_DEV_H */


-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] i2c-dev: Fix typo in ioctl name reference

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]

On Tue, Sep 08, 2015 at 11:05:49AM +0200, Jean Delvare wrote:
> The ioctl is named I2C_RDWR for "I2C read/write". But references to it
> were misspelled "rdrw". Fix them.
> 
> Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
> Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>

Wow! Amazing how long this went unnoticed/unfixed.

> -#define  I2C_RDRW_IOCTL_MAX_MSGS	42
> +#define  I2C_RDWR_IOCTL_MAX_MSGS	42
> +/* Originally defined with a typo, keep if for now for compatibility */

I would drop the 'for now' and keep it forever. A define doesn't hurt
and if it increases backwards compatibility, why not? I will also do
s/if/it/. No need to resend.

> +#define  I2C_RDRW_IOCTL_MAX_MSGS	I2C_RDWR_IOCTL_MAX_MSGS

Thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c-dev: Fix typo in ioctl name reference

[Jean Delvare]

On Thu, 10 Sep 2015 20:00:12 +0200, Wolfram Sang wrote:
> On Tue, Sep 08, 2015 at 11:05:49AM +0200, Jean Delvare wrote:
> > The ioctl is named I2C_RDWR for "I2C read/write". But references to it
> > were misspelled "rdrw". Fix them.
> > 
> > Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
> > Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> 
> Wow! Amazing how long this went unnoticed/unfixed.

Indeed :/

I don't think this constant was much (or ever) used in user-space
before i2ctransfer. That would be why.

> > -#define  I2C_RDRW_IOCTL_MAX_MSGS	42
> > +#define  I2C_RDWR_IOCTL_MAX_MSGS	42
> > +/* Originally defined with a typo, keep if for now for compatibility */
> 
> I would drop the 'for now' and keep it forever. A define doesn't hurt
> and if it increases backwards compatibility, why not? I will also do
> s/if/it/. No need to resend.

Sorry for the typo and thanks for catching it. "for now" or not is up
to you, my idea was that it probably wasn't used in user-space yet so
getting rid of it shouldn't hurt, while keeping it would encourage
people to use the wrong one instead of the good one. So on second
thought a better strategy may be to NOT keep the compatibility in the
first place.

It's not an ABI change anyway, and it's a minor thing really. Nobody is
ever going to hit this limit in practice, and if one does and did not
check beforehand, he/she will simply get -EINVAL from ioctl(), which
can be reported to the user just the same. IMHO I2C_RDWR_IOCTL_MAX_MSGS
should not have been made visible from user-space in the first place.

> > +#define  I2C_RDRW_IOCTL_MAX_MSGS	I2C_RDWR_IOCTL_MAX_MSGS

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] i2c-dev: Fix typo in ioctl name reference

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]

> > > -#define  I2C_RDRW_IOCTL_MAX_MSGS	42
> > > +#define  I2C_RDWR_IOCTL_MAX_MSGS	42
> > > +/* Originally defined with a typo, keep if for now for compatibility */
> > 
> > I would drop the 'for now' and keep it forever. A define doesn't hurt
> > and if it increases backwards compatibility, why not? I will also do
> > s/if/it/. No need to resend.
> 
> Sorry for the typo and thanks for catching it. "for now" or not is up
> to you, my idea was that it probably wasn't used in user-space yet so
> getting rid of it shouldn't hurt, while keeping it would encourage
> people to use the wrong one instead of the good one. So on second
> thought a better strategy may be to NOT keep the compatibility in the
> first place.

I really don't want to break userspace. And since this #define is in the
uapi headers, I rather be safe than sorry. There might be programs using
it we don't know about.

So:

Applied to for-next, thanks!

> can be reported to the user just the same. IMHO I2C_RDWR_IOCTL_MAX_MSGS
> should not have been made visible from user-space in the first place.

Yes.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]