[PATCH 0/7] staging: comedi: comedi_compat32.[ch] fix and tidy up

[PATCH 0/7] staging: comedi: comedi_compat32.[ch] fix and tidy up

[Ian Abbott]

Fix a bug in the handling of the 32-bit compatible version of the
COMEDI_CMD ioctl and tidy up the rest of the 32-bit compatible ioctl
handling code a bit.

1) staging: comedi: comedi_compat32.c: fix COMEDI_CMD copy back
2) staging: comedi: comedi_compat32.h: reformat copyright comment
3) staging: comedi: comedi_compat.c: reformat copyright comment
4) staging: comedi: comedi_compat32.c: reformat other block comments
5) staging: comedi: comedi_compat32.c: align some comments
6) staging: comedi: comedi_compat32.c: absorb raw_ioctl()
7) staging: comedi: comedi_compat.c: use long unlocked_ioctl return
   value

 drivers/staging/comedi/comedi_compat32.c | 121 +++++++++++++++++--------------
 drivers/staging/comedi/comedi_compat32.h |  38 +++++-----
 2 files changed, 85 insertions(+), 74 deletions(-)

[PATCH 1/7] staging: comedi: comedi_compat32.c: fix COMEDI_CMD copy back

[Ian Abbott]

`do_cmd_ioctl()` in "comedi_fops.c" handles the `COMEDI_CMD` ioctl.
This returns `-EAGAIN` if it has copied a modified `struct comedi_cmd`
back to user-space.  (This occurs when the low-level Comedi driver's
`do_cmdtest()` handler returns non-zero to indicate a problem with the
contents of the `struct comedi_cmd`, or when the `struct comedi_cmd` has
the `CMDF_BOGUS` flag set.)

`compat_cmd()` in "comedi_compat32.c" handles the 32-bit compatible
version of the `COMEDI_CMD` ioctl.  Currently, it never copies a 32-bit
compatible version of `struct comedi_cmd` back to user-space, which is
at odds with the way the regular `COMEDI_CMD` ioctl is handled.  To fix
it, change `compat_cmd()` to copy a 32-bit compatible version of the
`struct comedi_cmd` back to user-space when the main ioctl handler
returns `-EAGAIN`.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Cc: <stable@vger.kernel.org>
---
 drivers/staging/comedi/comedi_compat32.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
index 5a4c74f..2440c60 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -262,7 +262,8 @@ static int compat_cmd(struct file *file, unsigned long arg)
 {
 	struct comedi_cmd __user *cmd;
 	struct comedi32_cmd_struct __user *cmd32;
-	int rc;
+	long rc;
+	int err;
 
 	cmd32 = compat_ptr(arg);
 	cmd = compat_alloc_user_space(sizeof(*cmd));
@@ -271,7 +272,15 @@ static int compat_cmd(struct file *file, unsigned long arg)
 	if (rc)
 		return rc;
 
-	return translated_ioctl(file, COMEDI_CMD, (unsigned long)cmd);
+	rc = translated_ioctl(file, COMEDI_CMD, (unsigned long)cmd);
+	if (rc == -EAGAIN) {
+		/* Special case: copy cmd back to user. */
+		err = put_compat_cmd(cmd32, cmd);
+		if (err)
+			rc = err;
+	}
+
+	return rc;
 }
 
 /* Handle 32-bit COMEDI_CMDTEST ioctl. */
-- 
2.1.4

[PATCH 2/7] staging: comedi: comedi_compat32.h: reformat copyright comment

[Ian Abbott]

Use the usual block comment style.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/comedi_compat32.h | 38 ++++++++++++++++----------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.h b/drivers/staging/comedi/comedi_compat32.h
index 2d0a6fc..5ce77f3 100644
--- a/drivers/staging/comedi/comedi_compat32.h
+++ b/drivers/staging/comedi/comedi_compat32.h
@@ -1,23 +1,23 @@
 /*
-    comedi/comedi_compat32.h
-    32-bit ioctl compatibility for 64-bit comedi kernel module.
-
-    Author: Ian Abbott, MEV Ltd. <abbotti@mev.co.uk>
-    Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
-
-    COMEDI - Linux Control and Measurement Device Interface
-    Copyright (C) 1997-2007 David A. Schleef <ds@schleef.org>
-
-    This program is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-
-    This program is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-*/
+ * comedi/comedi_compat32.h
+ * 32-bit ioctl compatibility for 64-bit comedi kernel module.
+ *
+ * Author: Ian Abbott, MEV Ltd. <abbotti@mev.co.uk>
+ * Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 1997-2007 David A. Schleef <ds@schleef.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
 
 #ifndef _COMEDI_COMPAT32_H
 #define _COMEDI_COMPAT32_H
-- 
2.1.4

[PATCH 3/7] staging: comedi: comedi_compat.c: reformat copyright comment

[Ian Abbott]

Use the usual block comment style.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/comedi_compat32.c | 38 ++++++++++++++++----------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
index 2440c60..9364f08 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -1,23 +1,23 @@
 /*
-    comedi/comedi_compat32.c
-    32-bit ioctl compatibility for 64-bit comedi kernel module.
-
-    Author: Ian Abbott, MEV Ltd. <abbotti@mev.co.uk>
-    Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
-
-    COMEDI - Linux Control and Measurement Device Interface
-    Copyright (C) 1997-2007 David A. Schleef <ds@schleef.org>
-
-    This program is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-
-    This program is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-*/
+ * comedi/comedi_compat32.c
+ * 32-bit ioctl compatibility for 64-bit comedi kernel module.
+ *
+ * Author: Ian Abbott, MEV Ltd. <abbotti@mev.co.uk>
+ * Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 1997-2007 David A. Schleef <ds@schleef.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
 
 #include <linux/uaccess.h>
 #include <linux/compat.h>
-- 
2.1.4

[PATCH 4/7] staging: comedi: comedi_compat32.c: reformat other block comments

[Ian Abbott]

Use the usual block comment style.  Combine some consecutive comments
into block comments.  Also remove part of a comment referring to
`ptr_to_compat()` not being implemented until kernel version 2.6.11 as
it's irrelevant.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/comedi_compat32.c | 36 +++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
index 9364f08..c0d404c 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -27,11 +27,15 @@
 
 #define COMEDI32_CHANINFO _IOR(CIO, 3, struct comedi32_chaninfo_struct)
 #define COMEDI32_RANGEINFO _IOR(CIO, 8, struct comedi32_rangeinfo_struct)
-/* N.B. COMEDI32_CMD and COMEDI_CMD ought to use _IOWR, not _IOR.
- * It's too late to change it now, but it only affects the command number. */
+/*
+ * N.B. COMEDI32_CMD and COMEDI_CMD ought to use _IOWR, not _IOR.
+ * It's too late to change it now, but it only affects the command number.
+ */
 #define COMEDI32_CMD _IOR(CIO, 9, struct comedi32_cmd_struct)
-/* N.B. COMEDI32_CMDTEST and COMEDI_CMDTEST ought to use _IOWR, not _IOR.
- * It's too late to change it now, but it only affects the command number. */
+/*
+ * N.B. COMEDI32_CMDTEST and COMEDI_CMDTEST ought to use _IOWR, not _IOR.
+ * It's too late to change it now, but it only affects the command number.
+ */
 #define COMEDI32_CMDTEST _IOR(CIO, 10, struct comedi32_cmd_struct)
 #define COMEDI32_INSNLIST _IOR(CIO, 11, struct comedi32_insnlist_struct)
 #define COMEDI32_INSN _IOR(CIO, 12, struct comedi32_insn_struct)
@@ -215,10 +219,12 @@ static int put_compat_cmd(struct comedi32_cmd_struct __user *cmd32,
 	int err;
 	unsigned int temp;
 
-	/* Copy back most of cmd structure. */
-	/* Assume the pointer values are already valid. */
-	/* (Could use ptr_to_compat() to set them, but that wasn't implemented
-	 * until kernel version 2.6.11.) */
+	/*
+	 * Copy back most of cmd structure.
+	 *
+	 * Assume the pointer values are already valid.
+	 * (Could use ptr_to_compat() to set them.)
+	 */
 	if (!access_ok(VERIFY_READ, cmd, sizeof(*cmd)) ||
 	    !access_ok(VERIFY_WRITE, cmd32, sizeof(*cmd32)))
 		return -EFAULT;
@@ -404,8 +410,11 @@ static int compat_insn(struct file *file, unsigned long arg)
 	return translated_ioctl(file, COMEDI_INSN, (unsigned long)insn);
 }
 
-/* Process untranslated ioctl. */
-/* Returns -ENOIOCTLCMD for unrecognised ioctl codes. */
+/*
+ * Process untranslated ioctl.
+ *
+ * Returns -ENOIOCTLCMD for unrecognised ioctl codes.
+ */
 static inline int raw_ioctl(struct file *file, unsigned int cmd,
 			    unsigned long arg)
 {
@@ -455,8 +464,11 @@ static inline int raw_ioctl(struct file *file, unsigned int cmd,
 	return rc;
 }
 
-/* compat_ioctl file operation. */
-/* Returns -ENOIOCTLCMD for unrecognised ioctl codes. */
+/*
+ * compat_ioctl file operation.
+ *
+ * Returns -ENOIOCTLCMD for unrecognised ioctl codes.
+ */
 long comedi_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	return raw_ioctl(file, cmd, arg);
-- 
2.1.4

[PATCH 5/7] staging: comedi: comedi_compat32.c: align some comments

[Ian Abbott]

Align some comments attached to members of the 32-bit compatibility
structure definitions.  These comments describe the original pointer
types that are being represented by a `compat_uptr_t`.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/comedi_compat32.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
index c0d404c..88a784f 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -43,7 +43,7 @@
 struct comedi32_chaninfo_struct {
 	unsigned int subdev;
 	compat_uptr_t maxdata_list;	/* 32-bit 'unsigned int *' */
-	compat_uptr_t flaglist;	/* 32-bit 'unsigned int *' */
+	compat_uptr_t flaglist;		/* 32-bit 'unsigned int *' */
 	compat_uptr_t rangelist;	/* 32-bit 'unsigned int *' */
 	unsigned int unused[4];
 };
@@ -66,16 +66,16 @@ struct comedi32_cmd_struct {
 	unsigned int scan_end_arg;
 	unsigned int stop_src;
 	unsigned int stop_arg;
-	compat_uptr_t chanlist;	/* 32-bit 'unsigned int *' */
+	compat_uptr_t chanlist;		/* 32-bit 'unsigned int *' */
 	unsigned int chanlist_len;
-	compat_uptr_t data;	/* 32-bit 'short *' */
+	compat_uptr_t data;		/* 32-bit 'short *' */
 	unsigned int data_len;
 };
 
 struct comedi32_insn_struct {
 	unsigned int insn;
 	unsigned int n;
-	compat_uptr_t data;	/* 32-bit 'unsigned int *' */
+	compat_uptr_t data;		/* 32-bit 'unsigned int *' */
 	unsigned int subdev;
 	unsigned int chanspec;
 	unsigned int unused[3];
@@ -83,7 +83,7 @@ struct comedi32_insn_struct {
 
 struct comedi32_insnlist_struct {
 	unsigned int n_insns;
-	compat_uptr_t insns;	/* 32-bit 'struct comedi_insn *' */
+	compat_uptr_t insns;		/* 32-bit 'struct comedi_insn *' */
 };
 
 /* Handle translated ioctl. */
-- 
2.1.4

[PATCH 6/7] staging: comedi: comedi_compat32.c: absorb raw_ioctl()

[Ian Abbott]

`comedi_compat_ioctl()` just calls static inline function `raw_ioctl()`
with the same parameters (although the former returns a `long` and the
latter returns an `int`).  Since `raw_ioctl()` is not called from
anywhere else, just absorb its body into `comedi_compat_ioctl()`.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/comedi_compat32.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
index 88a784f..24078b9 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -411,12 +411,11 @@ static int compat_insn(struct file *file, unsigned long arg)
 }
 
 /*
- * Process untranslated ioctl.
+ * compat_ioctl file operation.
  *
  * Returns -ENOIOCTLCMD for unrecognised ioctl codes.
  */
-static inline int raw_ioctl(struct file *file, unsigned int cmd,
-			    unsigned long arg)
+long comedi_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	int rc;
 
@@ -463,13 +462,3 @@ static inline int raw_ioctl(struct file *file, unsigned int cmd,
 	}
 	return rc;
 }
-
-/*
- * compat_ioctl file operation.
- *
- * Returns -ENOIOCTLCMD for unrecognised ioctl codes.
- */
-long comedi_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	return raw_ioctl(file, cmd, arg);
-}
-- 
2.1.4

[PATCH 7/7] staging: comedi: comedi_compat.c: use long unlocked_ioctl return value

[Ian Abbott]

The `unlocked_ioctl` and `compat_ioctl` file operations are both defined
to return a `long` (I don't know why).  Comedi's `compat_ioctl` handler
`comedi_compat_ioctl()` and its helper functions currently assume the
`unlocked_ioctl` handler return value will fit in an `int` (which it
does, in fact).  Change it to pass through the full `long` return value.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/comedi_compat32.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
index 24078b9..06ccf3c 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -87,8 +87,8 @@ struct comedi32_insnlist_struct {
 };
 
 /* Handle translated ioctl. */
-static int translated_ioctl(struct file *file, unsigned int cmd,
-			    unsigned long arg)
+static long translated_ioctl(struct file *file, unsigned int cmd,
+			     unsigned long arg)
 {
 	if (file->f_op->unlocked_ioctl)
 		return file->f_op->unlocked_ioctl(file, cmd, arg);
@@ -97,7 +97,7 @@ static int translated_ioctl(struct file *file, unsigned int cmd,
 }
 
 /* Handle 32-bit COMEDI_CHANINFO ioctl. */
-static int compat_chaninfo(struct file *file, unsigned long arg)
+static long compat_chaninfo(struct file *file, unsigned long arg)
 {
 	struct comedi_chaninfo __user *chaninfo;
 	struct comedi32_chaninfo_struct __user *chaninfo32;
@@ -131,7 +131,7 @@ static int compat_chaninfo(struct file *file, unsigned long arg)
 }
 
 /* Handle 32-bit COMEDI_RANGEINFO ioctl. */
-static int compat_rangeinfo(struct file *file, unsigned long arg)
+static long compat_rangeinfo(struct file *file, unsigned long arg)
 {
 	struct comedi_rangeinfo __user *rangeinfo;
 	struct comedi32_rangeinfo_struct __user *rangeinfo32;
@@ -264,7 +264,7 @@ static int put_compat_cmd(struct comedi32_cmd_struct __user *cmd32,
 }
 
 /* Handle 32-bit COMEDI_CMD ioctl. */
-static int compat_cmd(struct file *file, unsigned long arg)
+static long compat_cmd(struct file *file, unsigned long arg)
 {
 	struct comedi_cmd __user *cmd;
 	struct comedi32_cmd_struct __user *cmd32;
@@ -290,11 +290,12 @@ static int compat_cmd(struct file *file, unsigned long arg)
 }
 
 /* Handle 32-bit COMEDI_CMDTEST ioctl. */
-static int compat_cmdtest(struct file *file, unsigned long arg)
+static long compat_cmdtest(struct file *file, unsigned long arg)
 {
 	struct comedi_cmd __user *cmd;
 	struct comedi32_cmd_struct __user *cmd32;
-	int rc, err;
+	long rc;
+	int err;
 
 	cmd32 = compat_ptr(arg);
 	cmd = compat_alloc_user_space(sizeof(*cmd));
@@ -344,7 +345,7 @@ static int get_compat_insn(struct comedi_insn __user *insn,
 }
 
 /* Handle 32-bit COMEDI_INSNLIST ioctl. */
-static int compat_insnlist(struct file *file, unsigned long arg)
+static long compat_insnlist(struct file *file, unsigned long arg)
 {
 	struct combined_insnlist {
 		struct comedi_insnlist insnlist;
@@ -394,7 +395,7 @@ static int compat_insnlist(struct file *file, unsigned long arg)
 }
 
 /* Handle 32-bit COMEDI_INSN ioctl. */
-static int compat_insn(struct file *file, unsigned long arg)
+static long compat_insn(struct file *file, unsigned long arg)
 {
 	struct comedi_insn __user *insn;
 	struct comedi32_insn_struct __user *insn32;
@@ -417,7 +418,7 @@ static int compat_insn(struct file *file, unsigned long arg)
  */
 long comedi_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
-	int rc;
+	long rc;
 
 	switch (cmd) {
 	case COMEDI_DEVCONFIG:
-- 
2.1.4

Re: [PATCH 1/7] staging: comedi: comedi_compat32.c: fix COMEDI_CMD copy back

[Ian Abbott]

On 27/01/15 15:50, Ian Abbott wrote:
> `do_cmd_ioctl()` in "comedi_fops.c" handles the `COMEDI_CMD` ioctl.
> This returns `-EAGAIN` if it has copied a modified `struct comedi_cmd`
> back to user-space.  (This occurs when the low-level Comedi driver's
> `do_cmdtest()` handler returns non-zero to indicate a problem with the
> contents of the `struct comedi_cmd`, or when the `struct comedi_cmd` has
> the `CMDF_BOGUS` flag set.)
>
> `compat_cmd()` in "comedi_compat32.c" handles the 32-bit compatible
> version of the `COMEDI_CMD` ioctl.  Currently, it never copies a 32-bit
> compatible version of `struct comedi_cmd` back to user-space, which is
> at odds with the way the regular `COMEDI_CMD` ioctl is handled.  To fix
> it, change `compat_cmd()` to copy a 32-bit compatible version of the
> `struct comedi_cmd` back to user-space when the main ioctl handler
> returns `-EAGAIN`.
>
> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> Cc: <stable@vger.kernel.org>
> ---
>   drivers/staging/comedi/comedi_compat32.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
> index 5a4c74f..2440c60 100644
> --- a/drivers/staging/comedi/comedi_compat32.c
> +++ b/drivers/staging/comedi/comedi_compat32.c
> @@ -262,7 +262,8 @@ static int compat_cmd(struct file *file, unsigned long arg)
>   {
>   	struct comedi_cmd __user *cmd;
>   	struct comedi32_cmd_struct __user *cmd32;
> -	int rc;
> +	long rc;
> +	int err;

Gah!  That change in type of the 'rc' variable resulted from me changing 
the order of the patches in the series.  It still works, but looks a bit 
out of place.  Should I post an updated version without this niggle?

>
>   	cmd32 = compat_ptr(arg);
>   	cmd = compat_alloc_user_space(sizeof(*cmd));
> @@ -271,7 +272,15 @@ static int compat_cmd(struct file *file, unsigned long arg)
>   	if (rc)
>   		return rc;
>
> -	return translated_ioctl(file, COMEDI_CMD, (unsigned long)cmd);
> +	rc = translated_ioctl(file, COMEDI_CMD, (unsigned long)cmd);
> +	if (rc == -EAGAIN) {
> +		/* Special case: copy cmd back to user. */
> +		err = put_compat_cmd(cmd32, cmd);
> +		if (err)
> +			rc = err;
> +	}
> +
> +	return rc;
>   }
>
>   /* Handle 32-bit COMEDI_CMDTEST ioctl. */
>


-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-

RE: [PATCH 1/7] staging: comedi: comedi_compat32.c: fix COMEDI_CMD copy back

[Hartley Sweeten]

On Tuesday, January 27, 2015 8:59 AM, Ian Abbott wrote:
> On 27/01/15 15:50, Ian Abbott wrote:
>> `do_cmd_ioctl()` in "comedi_fops.c" handles the `COMEDI_CMD` ioctl.
>> This returns `-EAGAIN` if it has copied a modified `struct comedi_cmd`
>> back to user-space.  (This occurs when the low-level Comedi driver's
>> `do_cmdtest()` handler returns non-zero to indicate a problem with the
>> contents of the `struct comedi_cmd`, or when the `struct comedi_cmd` has
>> the `CMDF_BOGUS` flag set.)
>>
>> `compat_cmd()` in "comedi_compat32.c" handles the 32-bit compatible
>> version of the `COMEDI_CMD` ioctl.  Currently, it never copies a 32-bit
>> compatible version of `struct comedi_cmd` back to user-space, which is
>> at odds with the way the regular `COMEDI_CMD` ioctl is handled.  To fix
>> it, change `compat_cmd()` to copy a 32-bit compatible version of the
>> `struct comedi_cmd` back to user-space when the main ioctl handler
>> returns `-EAGAIN`.
>>
>> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
>> Cc: <stable@vger.kernel.org>
>> ---
>>   drivers/staging/comedi/comedi_compat32.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
>> index 5a4c74f..2440c60 100644
>> --- a/drivers/staging/comedi/comedi_compat32.c
>> +++ b/drivers/staging/comedi/comedi_compat32.c
>> @@ -262,7 +262,8 @@ static int compat_cmd(struct file *file, unsigned long arg)
>>   {
>>   	struct comedi_cmd __user *cmd;
>>   	struct comedi32_cmd_struct __user *cmd32;
>> -	int rc;
>> +	long rc;
>> +	int err;
>
> Gah!  That change in type of the 'rc' variable resulted from me changing 
> the order of the patches in the series.  It still works, but looks a bit 
> out of place.  Should I post an updated version without this niggle?

Please fix it. I does look strange. Actually, the last patch looks strange.

The "normal" return type in the kernel is an 'int'. As you mention in the
commit message: "The `unlocked_ioctl` and `compat_ioctl` file operations
are both defined to return a `long` (I don't know why)." It seems cleaner
to just have all the static functions return an int and just have
comedi_compat_ioctl() return the long value. Maybe just add a comment
why...

My 2 cents...

Regards,
Hartley

Re: [PATCH 1/7] staging: comedi: comedi_compat32.c: fix COMEDI_CMD copy back

[Ian Abbott]

On 27/01/15 17:20, Hartley Sweeten wrote:
> On Tuesday, January 27, 2015 8:59 AM, Ian Abbott wrote:
>> On 27/01/15 15:50, Ian Abbott wrote:
>>> `do_cmd_ioctl()` in "comedi_fops.c" handles the `COMEDI_CMD` ioctl.
>>> This returns `-EAGAIN` if it has copied a modified `struct comedi_cmd`
>>> back to user-space.  (This occurs when the low-level Comedi driver's
>>> `do_cmdtest()` handler returns non-zero to indicate a problem with the
>>> contents of the `struct comedi_cmd`, or when the `struct comedi_cmd` has
>>> the `CMDF_BOGUS` flag set.)
>>>
>>> `compat_cmd()` in "comedi_compat32.c" handles the 32-bit compatible
>>> version of the `COMEDI_CMD` ioctl.  Currently, it never copies a 32-bit
>>> compatible version of `struct comedi_cmd` back to user-space, which is
>>> at odds with the way the regular `COMEDI_CMD` ioctl is handled.  To fix
>>> it, change `compat_cmd()` to copy a 32-bit compatible version of the
>>> `struct comedi_cmd` back to user-space when the main ioctl handler
>>> returns `-EAGAIN`.
>>>
>>> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
>>> Cc: <stable@vger.kernel.org>
>>> ---
>>>    drivers/staging/comedi/comedi_compat32.c | 13 +++++++++++--
>>>    1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
>>> index 5a4c74f..2440c60 100644
>>> --- a/drivers/staging/comedi/comedi_compat32.c
>>> +++ b/drivers/staging/comedi/comedi_compat32.c
>>> @@ -262,7 +262,8 @@ static int compat_cmd(struct file *file, unsigned long arg)
>>>    {
>>>    	struct comedi_cmd __user *cmd;
>>>    	struct comedi32_cmd_struct __user *cmd32;
>>> -	int rc;
>>> +	long rc;
>>> +	int err;
>>
>> Gah!  That change in type of the 'rc' variable resulted from me changing
>> the order of the patches in the series.  It still works, but looks a bit
>> out of place.  Should I post an updated version without this niggle?
>
> Please fix it. I does look strange. Actually, the last patch looks strange.
>
> The "normal" return type in the kernel is an 'int'. As you mention in the
> commit message: "The `unlocked_ioctl` and `compat_ioctl` file operations
> are both defined to return a `long` (I don't know why)." It seems cleaner
> to just have all the static functions return an int and just have
> comedi_compat_ioctl() return the long value. Maybe just add a comment
> why...
>
> My 2 cents...

Yes, on reflection I think passing through the `long` return value is 
just pandering to an abomination that should be ignored.  I'll post a v2 
series with the above niggle fixed and the final patch dropped.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-

[PATCH v2 0/6] staging: comedi: comedi_compat32.[ch] fix and tidy up

[Ian Abbott]

Fix a bug in the handling of the 32-bit compatible version of the
COMEDI_CMD ioctl and tidy up the rest of the 32-bit compatible ioctl
handling code a bit.

v2: fix minor niggle in patch 1 and drop patch 7.

1) staging: comedi: comedi_compat32.c: fix COMEDI_CMD copy back
2) staging: comedi: comedi_compat32.h: reformat copyright comment
3) staging: comedi: comedi_compat.c: reformat copyright comment
4) staging: comedi: comedi_compat32.c: reformat other block comments
5) staging: comedi: comedi_compat32.c: align some comments
6) staging: comedi: comedi_compat32.c: absorb raw_ioctl()

 drivers/staging/comedi/comedi_compat32.c | 99 +++++++++++++++++---------------
 drivers/staging/comedi/comedi_compat32.h | 38 ++++++------
 2 files changed, 73 insertions(+), 64 deletions(-)

[PATCH v2 1/6] staging: comedi: comedi_compat32.c: fix COMEDI_CMD copy back

[Ian Abbott]

`do_cmd_ioctl()` in "comedi_fops.c" handles the `COMEDI_CMD` ioctl.
This returns `-EAGAIN` if it has copied a modified `struct comedi_cmd`
back to user-space.  (This occurs when the low-level Comedi driver's
`do_cmdtest()` handler returns non-zero to indicate a problem with the
contents of the `struct comedi_cmd`, or when the `struct comedi_cmd` has
the `CMDF_BOGUS` flag set.)

`compat_cmd()` in "comedi_compat32.c" handles the 32-bit compatible
version of the `COMEDI_CMD` ioctl.  Currently, it never copies a 32-bit
compatible version of `struct comedi_cmd` back to user-space, which is
at odds with the way the regular `COMEDI_CMD` ioctl is handled.  To fix
it, change `compat_cmd()` to copy a 32-bit compatible version of the
`struct comedi_cmd` back to user-space when the main ioctl handler
returns `-EAGAIN`.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Cc: <stable@vger.kernel.org>
---
v2: keep existing type of `rc` variable - don't change it to `long`.
---
 drivers/staging/comedi/comedi_compat32.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
index 5a4c74f..03a2d07 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -262,7 +262,7 @@ static int compat_cmd(struct file *file, unsigned long arg)
 {
 	struct comedi_cmd __user *cmd;
 	struct comedi32_cmd_struct __user *cmd32;
-	int rc;
+	int rc, err;
 
 	cmd32 = compat_ptr(arg);
 	cmd = compat_alloc_user_space(sizeof(*cmd));
@@ -271,7 +271,15 @@ static int compat_cmd(struct file *file, unsigned long arg)
 	if (rc)
 		return rc;
 
-	return translated_ioctl(file, COMEDI_CMD, (unsigned long)cmd);
+	rc = translated_ioctl(file, COMEDI_CMD, (unsigned long)cmd);
+	if (rc == -EAGAIN) {
+		/* Special case: copy cmd back to user. */
+		err = put_compat_cmd(cmd32, cmd);
+		if (err)
+			rc = err;
+	}
+
+	return rc;
 }
 
 /* Handle 32-bit COMEDI_CMDTEST ioctl. */
-- 
2.1.4

[PATCH v2 2/6] staging: comedi: comedi_compat32.h: reformat copyright comment

[Ian Abbott]

Use the usual block comment style.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
v2: no change
---
 drivers/staging/comedi/comedi_compat32.h | 38 ++++++++++++++++----------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.h b/drivers/staging/comedi/comedi_compat32.h
index 2d0a6fc..5ce77f3 100644
--- a/drivers/staging/comedi/comedi_compat32.h
+++ b/drivers/staging/comedi/comedi_compat32.h
@@ -1,23 +1,23 @@
 /*
-    comedi/comedi_compat32.h
-    32-bit ioctl compatibility for 64-bit comedi kernel module.
-
-    Author: Ian Abbott, MEV Ltd. <abbotti@mev.co.uk>
-    Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
-
-    COMEDI - Linux Control and Measurement Device Interface
-    Copyright (C) 1997-2007 David A. Schleef <ds@schleef.org>
-
-    This program is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-
-    This program is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-*/
+ * comedi/comedi_compat32.h
+ * 32-bit ioctl compatibility for 64-bit comedi kernel module.
+ *
+ * Author: Ian Abbott, MEV Ltd. <abbotti@mev.co.uk>
+ * Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 1997-2007 David A. Schleef <ds@schleef.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
 
 #ifndef _COMEDI_COMPAT32_H
 #define _COMEDI_COMPAT32_H
-- 
2.1.4

[PATCH v2 3/6] staging: comedi: comedi_compat.c: reformat copyright comment

[Ian Abbott]

Use the usual block comment style.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
v2: no change
---
 drivers/staging/comedi/comedi_compat32.c | 38 ++++++++++++++++----------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
index 03a2d07..097fd4d 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -1,23 +1,23 @@
 /*
-    comedi/comedi_compat32.c
-    32-bit ioctl compatibility for 64-bit comedi kernel module.
-
-    Author: Ian Abbott, MEV Ltd. <abbotti@mev.co.uk>
-    Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
-
-    COMEDI - Linux Control and Measurement Device Interface
-    Copyright (C) 1997-2007 David A. Schleef <ds@schleef.org>
-
-    This program is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-
-    This program is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-*/
+ * comedi/comedi_compat32.c
+ * 32-bit ioctl compatibility for 64-bit comedi kernel module.
+ *
+ * Author: Ian Abbott, MEV Ltd. <abbotti@mev.co.uk>
+ * Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 1997-2007 David A. Schleef <ds@schleef.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
 
 #include <linux/uaccess.h>
 #include <linux/compat.h>
-- 
2.1.4

[PATCH v2 4/6] staging: comedi: comedi_compat32.c: reformat other block comments

[Ian Abbott]

Use the usual block comment style.  Combine some consecutive comments
into block comments.  Also remove part of a comment referring to
`ptr_to_compat()` not being implemented until kernel version 2.6.11 as
it's irrelevant.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
v2: no change
---
 drivers/staging/comedi/comedi_compat32.c | 36 +++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
index 097fd4d..acbacff 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -27,11 +27,15 @@
 
 #define COMEDI32_CHANINFO _IOR(CIO, 3, struct comedi32_chaninfo_struct)
 #define COMEDI32_RANGEINFO _IOR(CIO, 8, struct comedi32_rangeinfo_struct)
-/* N.B. COMEDI32_CMD and COMEDI_CMD ought to use _IOWR, not _IOR.
- * It's too late to change it now, but it only affects the command number. */
+/*
+ * N.B. COMEDI32_CMD and COMEDI_CMD ought to use _IOWR, not _IOR.
+ * It's too late to change it now, but it only affects the command number.
+ */
 #define COMEDI32_CMD _IOR(CIO, 9, struct comedi32_cmd_struct)
-/* N.B. COMEDI32_CMDTEST and COMEDI_CMDTEST ought to use _IOWR, not _IOR.
- * It's too late to change it now, but it only affects the command number. */
+/*
+ * N.B. COMEDI32_CMDTEST and COMEDI_CMDTEST ought to use _IOWR, not _IOR.
+ * It's too late to change it now, but it only affects the command number.
+ */
 #define COMEDI32_CMDTEST _IOR(CIO, 10, struct comedi32_cmd_struct)
 #define COMEDI32_INSNLIST _IOR(CIO, 11, struct comedi32_insnlist_struct)
 #define COMEDI32_INSN _IOR(CIO, 12, struct comedi32_insn_struct)
@@ -215,10 +219,12 @@ static int put_compat_cmd(struct comedi32_cmd_struct __user *cmd32,
 	int err;
 	unsigned int temp;
 
-	/* Copy back most of cmd structure. */
-	/* Assume the pointer values are already valid. */
-	/* (Could use ptr_to_compat() to set them, but that wasn't implemented
-	 * until kernel version 2.6.11.) */
+	/*
+	 * Copy back most of cmd structure.
+	 *
+	 * Assume the pointer values are already valid.
+	 * (Could use ptr_to_compat() to set them.)
+	 */
 	if (!access_ok(VERIFY_READ, cmd, sizeof(*cmd)) ||
 	    !access_ok(VERIFY_WRITE, cmd32, sizeof(*cmd32)))
 		return -EFAULT;
@@ -403,8 +409,11 @@ static int compat_insn(struct file *file, unsigned long arg)
 	return translated_ioctl(file, COMEDI_INSN, (unsigned long)insn);
 }
 
-/* Process untranslated ioctl. */
-/* Returns -ENOIOCTLCMD for unrecognised ioctl codes. */
+/*
+ * Process untranslated ioctl.
+ *
+ * Returns -ENOIOCTLCMD for unrecognised ioctl codes.
+ */
 static inline int raw_ioctl(struct file *file, unsigned int cmd,
 			    unsigned long arg)
 {
@@ -454,8 +463,11 @@ static inline int raw_ioctl(struct file *file, unsigned int cmd,
 	return rc;
 }
 
-/* compat_ioctl file operation. */
-/* Returns -ENOIOCTLCMD for unrecognised ioctl codes. */
+/*
+ * compat_ioctl file operation.
+ *
+ * Returns -ENOIOCTLCMD for unrecognised ioctl codes.
+ */
 long comedi_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	return raw_ioctl(file, cmd, arg);
-- 
2.1.4

[PATCH v2 5/6] staging: comedi: comedi_compat32.c: align some comments

[Ian Abbott]

Align some comments attached to members of the 32-bit compatibility
structure definitions.  These comments describe the original pointer
types that are being represented by a `compat_uptr_t`.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
v2: no change
---
 drivers/staging/comedi/comedi_compat32.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
index acbacff..0eab6c6 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -43,7 +43,7 @@
 struct comedi32_chaninfo_struct {
 	unsigned int subdev;
 	compat_uptr_t maxdata_list;	/* 32-bit 'unsigned int *' */
-	compat_uptr_t flaglist;	/* 32-bit 'unsigned int *' */
+	compat_uptr_t flaglist;		/* 32-bit 'unsigned int *' */
 	compat_uptr_t rangelist;	/* 32-bit 'unsigned int *' */
 	unsigned int unused[4];
 };
@@ -66,16 +66,16 @@ struct comedi32_cmd_struct {
 	unsigned int scan_end_arg;
 	unsigned int stop_src;
 	unsigned int stop_arg;
-	compat_uptr_t chanlist;	/* 32-bit 'unsigned int *' */
+	compat_uptr_t chanlist;		/* 32-bit 'unsigned int *' */
 	unsigned int chanlist_len;
-	compat_uptr_t data;	/* 32-bit 'short *' */
+	compat_uptr_t data;		/* 32-bit 'short *' */
 	unsigned int data_len;
 };
 
 struct comedi32_insn_struct {
 	unsigned int insn;
 	unsigned int n;
-	compat_uptr_t data;	/* 32-bit 'unsigned int *' */
+	compat_uptr_t data;		/* 32-bit 'unsigned int *' */
 	unsigned int subdev;
 	unsigned int chanspec;
 	unsigned int unused[3];
@@ -83,7 +83,7 @@ struct comedi32_insn_struct {
 
 struct comedi32_insnlist_struct {
 	unsigned int n_insns;
-	compat_uptr_t insns;	/* 32-bit 'struct comedi_insn *' */
+	compat_uptr_t insns;		/* 32-bit 'struct comedi_insn *' */
 };
 
 /* Handle translated ioctl. */
-- 
2.1.4

[PATCH v2 6/6] staging: comedi: comedi_compat32.c: absorb raw_ioctl()

[Ian Abbott]

`comedi_compat_ioctl()` just calls static inline function `raw_ioctl()`
with the same parameters (although the former returns a `long` and the
latter returns an `int`).  Since `raw_ioctl()` is not called from
anywhere else, just absorb its body into `comedi_compat_ioctl()`.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
v2: no change
---
 drivers/staging/comedi/comedi_compat32.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
index 0eab6c6..2584824 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -410,12 +410,11 @@ static int compat_insn(struct file *file, unsigned long arg)
 }
 
 /*
- * Process untranslated ioctl.
+ * compat_ioctl file operation.
  *
  * Returns -ENOIOCTLCMD for unrecognised ioctl codes.
  */
-static inline int raw_ioctl(struct file *file, unsigned int cmd,
-			    unsigned long arg)
+long comedi_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	int rc;
 
@@ -462,13 +461,3 @@ static inline int raw_ioctl(struct file *file, unsigned int cmd,
 	}
 	return rc;
 }
-
-/*
- * compat_ioctl file operation.
- *
- * Returns -ENOIOCTLCMD for unrecognised ioctl codes.
- */
-long comedi_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	return raw_ioctl(file, cmd, arg);
-}
-- 
2.1.4

Re: [PATCH v2 3/6] staging: comedi: comedi_compat.c: reformat copyright comment

[Ian Abbott]

Bugger!  That should be "comedi_compat32.c", not "comedi_compat.c".

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-

[PATCH v3 3/6] staging: comedi: comedi_compat32.c: reformat copyright comment

[Ian Abbott]

Use the usual block comment style.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
v2: no change
v3: corrected commit summary line
---
 drivers/staging/comedi/comedi_compat32.c | 38 ++++++++++++++++----------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
index 03a2d07..097fd4d 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -1,23 +1,23 @@
 /*
-    comedi/comedi_compat32.c
-    32-bit ioctl compatibility for 64-bit comedi kernel module.
-
-    Author: Ian Abbott, MEV Ltd. <abbotti@mev.co.uk>
-    Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
-
-    COMEDI - Linux Control and Measurement Device Interface
-    Copyright (C) 1997-2007 David A. Schleef <ds@schleef.org>
-
-    This program is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-
-    This program is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-*/
+ * comedi/comedi_compat32.c
+ * 32-bit ioctl compatibility for 64-bit comedi kernel module.
+ *
+ * Author: Ian Abbott, MEV Ltd. <abbotti@mev.co.uk>
+ * Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 1997-2007 David A. Schleef <ds@schleef.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
 
 #include <linux/uaccess.h>
 #include <linux/compat.h>
-- 
2.1.4

RE: [PATCH v2 0/6] staging: comedi: comedi_compat32.[ch] fix and tidy up

[Hartley Sweeten]

On Tuesday, January 27, 2015 11:17 AM, Ian Abbott wrote:
> Fix a bug in the handling of the 32-bit compatible version of the
> COMEDI_CMD ioctl and tidy up the rest of the 32-bit compatible ioctl
> handling code a bit.
>
> v2: fix minor niggle in patch 1 and drop patch 7.
>
> 1) staging: comedi: comedi_compat32.c: fix COMEDI_CMD copy back
> 2) staging: comedi: comedi_compat32.h: reformat copyright comment
> 3) staging: comedi: comedi_compat.c: reformat copyright comment
> 4) staging: comedi: comedi_compat32.c: reformat other block comments
> 5) staging: comedi: comedi_compat32.c: align some comments
> 6) staging: comedi: comedi_compat32.c: absorb raw_ioctl()
>
>  drivers/staging/comedi/comedi_compat32.c | 99 +++++++++++++++++---------------
>  drivers/staging/comedi/comedi_compat32.h | 38 ++++++------
>  2 files changed, 73 insertions(+), 64 deletions(-)

Looks better. Thanks.

Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>