[PATCH v2] usb: gadget: ffs: Fix sparse error

[PATCH v2] usb: gadget: ffs: Fix sparse error

[Rohith Seelaboyina]

This patch fixes the sparse error in functionfs
driver.

drivers/usb/gadget/function/f_fs.c:400:44: error: bad
constant experssion.

Dynamic memory allocation through kcalloc is more safer
than declaring variable array size, Fix this error by
using kcalloc for memory allocation, Check if memory
allocation is successful and return -ENOMEM on failure.

Signed-off-by: Rohith Seelaboyina <rseelaboyina@nvidia.com>
---
Changes in v2:
   - Use kcalloc to allocate memory instead of using
     kmalloc and memset.

 drivers/usb/gadget/function/f_fs.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 63314ede7ba6..a4858db41f5f 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -397,10 +397,13 @@ static ssize_t __ffs_ep0_read_events(struct ffs_data *ffs, char __user *buf,
 	 * We are holding ffs->ev.waitq.lock and ffs->mutex and we need
 	 * to release them.
 	 */
-	struct usb_functionfs_event events[n];
 	unsigned i = 0;
+	int ret;
+	struct usb_functionfs_event *events = kcalloc(n,
+			sizeof(struct usb_functionfs_event), GFP_KERNEL);
 
-	memset(events, 0, sizeof events);
+	if (unlikely(!events))
+		return -ENOMEM;
 
 	do {
 		events[i].type = ffs->ev.types[i];
@@ -421,8 +424,10 @@ static ssize_t __ffs_ep0_read_events(struct ffs_data *ffs, char __user *buf,
 	spin_unlock_irq(&ffs->ev.waitq.lock);
 	mutex_unlock(&ffs->mutex);
 
-	return unlikely(__copy_to_user(buf, events, sizeof events))
-		? -EFAULT : sizeof events;
+	ret = unlikely(__copy_to_user(buf, events, n * sizeof(*events)))
+		? -EFAULT : n * sizeof(*events);
+	kfree(events);
+	return ret;
 }
 
 static ssize_t ffs_ep0_read(struct file *file, char __user *buf,
-- 
1.9.1

Re: [PATCH v2] usb: gadget: ffs: Fix sparse error

[Joe Perches]

On Wed, 2014-12-24 at 14:48 +0530, Rohith Seelaboyina wrote:
> Dynamic memory allocation through kcalloc is more safer
> than declaring variable array size, Fix this error by
> using kcalloc for memory allocation, Check if memory
> allocation is successful and return -ENOMEM on failure.
> 
> Signed-off-by: Rohith Seelaboyina <rseelaboyina@nvidia.com>
> ---
> Changes in v2:
>    - Use kcalloc to allocate memory instead of using
>      kmalloc and memset.

style trivia:

> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
[]
> @@ -397,10 +397,13 @@ static ssize_t __ffs_ep0_read_events(struct ffs_data *ffs, char __user *buf,
>  	 * We are holding ffs->ev.waitq.lock and ffs->mutex and we need
>  	 * to release them.
>  	 */
> -	struct usb_functionfs_event events[n];
>  	unsigned i = 0;
> +	int ret;
> +	struct usb_functionfs_event *events = kcalloc(n,
> +			sizeof(struct usb_functionfs_event), GFP_KERNEL);
>  
> -	memset(events, 0, sizeof events);
> +	if (unlikely(!events))
> +		return -ENOMEM;

unlikely is not commonly used here.

I might be better to separate the declaration and the alloc
{
	[...]
	struct usb_functionfs_event *events;

	events = kcalloc(n, sizeof(*events), GFP_KERNEL);
	if (!events)
		return -ENOMEM;