diff mbox

[OPW,kernel] staging: comedi: use memdup_user to simplify code

Message ID 1382700201-3088-1-git-send-email-teobaluta@gmail.com
State New, archived
Headers show

Commit Message

Teodora Baluta Oct. 25, 2013, 11:23 a.m. UTC
Use memdup_user rather than duplicating implementation. Fix following
coccinelle warnings:

drivers/staging/comedi/comedi_fops.c:1425:5-12: WARNING opportunity for memdup_user
drivers/staging/comedi/comedi_fops.c:1553:6-13: WARNING opportunity for memdup_user

Signed-off-by: Teodora Baluta <teobaluta@gmail.com>
---
 drivers/staging/comedi/comedi_fops.c |   33 ++++++++++-----------------------
 1 file changed, 10 insertions(+), 23 deletions(-)

Comments

Greg KH Oct. 28, 2013, 9:44 p.m. UTC | #1
On Fri, Oct 25, 2013 at 02:23:21PM +0300, Teodora Baluta wrote:
> Use memdup_user rather than duplicating implementation. Fix following
> coccinelle warnings:
> 
> drivers/staging/comedi/comedi_fops.c:1425:5-12: WARNING opportunity for memdup_user
> drivers/staging/comedi/comedi_fops.c:1553:6-13: WARNING opportunity for memdup_user
> 
> Signed-off-by: Teodora Baluta <teobaluta@gmail.com>
> ---
>  drivers/staging/comedi/comedi_fops.c |   33 ++++++++++-----------------------
>  1 file changed, 10 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
> index 721df31..d2b6b84 100644
> --- a/drivers/staging/comedi/comedi_fops.c
> +++ b/drivers/staging/comedi/comedi_fops.c
> @@ -1421,17 +1421,11 @@ static int do_cmd_ioctl(struct comedi_device *dev,
>  	async->cmd = cmd;
>  	async->cmd.data = NULL;
>  	/* load channel/gain list */
> -	async->cmd.chanlist =
> -	    kmalloc(async->cmd.chanlist_len * sizeof(int), GFP_KERNEL);
> -	if (!async->cmd.chanlist) {
> -		DPRINTK("allocation failed\n");
> -		return -ENOMEM;
> -	}
> -
> -	if (copy_from_user(async->cmd.chanlist, user_chanlist,
> -			   async->cmd.chanlist_len * sizeof(int))) {
> -		DPRINTK("fault reading chanlist\n");
> -		ret = -EFAULT;
> +	async->cmd.chanlist = memdup_user(
> +	    user_chanlist, async->cmd.chanlist_len * sizeof(int));

Don't have "empty" ( functions, please use something like:

	async->cmd.chanlist = memdup_user(user_chanlist,
					  async->cmd.chanlist_len * sizeof(int));

Can you please redo this?

thanks,

greg k-h
Teodora Baluta Oct. 28, 2013, 9:58 p.m. UTC | #2
On Mon, Oct 28, 2013 at 11:44 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Oct 25, 2013 at 02:23:21PM +0300, Teodora Baluta wrote:
>> Use memdup_user rather than duplicating implementation. Fix following
>> coccinelle warnings:
>>
>> drivers/staging/comedi/comedi_fops.c:1425:5-12: WARNING opportunity for memdup_user
>> drivers/staging/comedi/comedi_fops.c:1553:6-13: WARNING opportunity for memdup_user
>>
>> Signed-off-by: Teodora Baluta <teobaluta@gmail.com>
>> ---
>>  drivers/staging/comedi/comedi_fops.c |   33 ++++++++++-----------------------
>>  1 file changed, 10 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
>> index 721df31..d2b6b84 100644
>> --- a/drivers/staging/comedi/comedi_fops.c
>> +++ b/drivers/staging/comedi/comedi_fops.c
>> @@ -1421,17 +1421,11 @@ static int do_cmd_ioctl(struct comedi_device *dev,
>>       async->cmd = cmd;
>>       async->cmd.data = NULL;
>>       /* load channel/gain list */
>> -     async->cmd.chanlist =
>> -         kmalloc(async->cmd.chanlist_len * sizeof(int), GFP_KERNEL);
>> -     if (!async->cmd.chanlist) {
>> -             DPRINTK("allocation failed\n");
>> -             return -ENOMEM;
>> -     }
>> -
>> -     if (copy_from_user(async->cmd.chanlist, user_chanlist,
>> -                        async->cmd.chanlist_len * sizeof(int))) {
>> -             DPRINTK("fault reading chanlist\n");
>> -             ret = -EFAULT;
>> +     async->cmd.chanlist = memdup_user(
>> +         user_chanlist, async->cmd.chanlist_len * sizeof(int));
>
> Don't have "empty" ( functions, please use something like:
>
>         async->cmd.chanlist = memdup_user(user_chanlist,
>                                           async->cmd.chanlist_len * sizeof(int));
>
> Can you please redo this?
>
> thanks,
>
> greg k-h

I'll send a patch v2 asap.

Thanks,
Teodora
diff mbox

Patch

diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index 721df31..d2b6b84 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1421,17 +1421,11 @@  static int do_cmd_ioctl(struct comedi_device *dev,
 	async->cmd = cmd;
 	async->cmd.data = NULL;
 	/* load channel/gain list */
-	async->cmd.chanlist =
-	    kmalloc(async->cmd.chanlist_len * sizeof(int), GFP_KERNEL);
-	if (!async->cmd.chanlist) {
-		DPRINTK("allocation failed\n");
-		return -ENOMEM;
-	}
-
-	if (copy_from_user(async->cmd.chanlist, user_chanlist,
-			   async->cmd.chanlist_len * sizeof(int))) {
-		DPRINTK("fault reading chanlist\n");
-		ret = -EFAULT;
+	async->cmd.chanlist = memdup_user(
+	    user_chanlist, async->cmd.chanlist_len * sizeof(int));
+	if (IS_ERR(async->cmd.chanlist)) {
+		ret = PTR_ERR(async->cmd.chanlist);
+		DPRINTK("memdup_user failed with code %d\n", ret);
 		goto cleanup;
 	}
 
@@ -1549,18 +1543,11 @@  static int do_cmdtest_ioctl(struct comedi_device *dev,
 
 	/* load channel/gain list */
 	if (cmd.chanlist) {
-		chanlist =
-		    kmalloc(cmd.chanlist_len * sizeof(int), GFP_KERNEL);
-		if (!chanlist) {
-			DPRINTK("allocation failed\n");
-			ret = -ENOMEM;
-			goto cleanup;
-		}
-
-		if (copy_from_user(chanlist, user_chanlist,
-				   cmd.chanlist_len * sizeof(int))) {
-			DPRINTK("fault reading chanlist\n");
-			ret = -EFAULT;
+		chanlist = memdup_user(user_chanlist,
+				       cmd.chanlist_len * sizeof(int));
+		if (IS_ERR(chanlist)) {
+			ret = PTR_ERR(chanlist);
+			DPRINTK("memdup_user exited with code %d", ret);
 			goto cleanup;
 		}