2010-07-31 17:38:46

by Kulikov Vasiliy

[permalink] [raw]
Subject: [PATCH 2/7] usb: fsl_udc_core: check return value of create_proc_read_entry()

create_proc_read_entry() may fail, if so return -ENOMEM.

Signed-off-by: Kulikov Vasiliy <[email protected]>
---
drivers/usb/gadget/fsl_udc_core.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index 08a9a62..c3d1545 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2128,7 +2128,7 @@ static int fsl_proc_read(char *page, char **start, off_t off, int count,

#else /* !CONFIG_USB_GADGET_DEBUG_FILES */

-#define create_proc_file() do {} while (0)
+#define create_proc_file() ({ (void *)1; })
#define remove_proc_file() do {} while (0)

#endif /* CONFIG_USB_GADGET_DEBUG_FILES */
@@ -2373,9 +2373,14 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
ret = -ENOMEM;
goto err_unregister;
}
- create_proc_file();
+ if (create_proc_file() == 0) {
+ ret = -ENOMEM;
+ goto err_pool;
+ }
return 0;

+err_pool:
+ dma_pool_destroy(udc_controller->td_pool);
err_unregister:
device_unregister(&udc_controller->gadget.dev);
err_free_irq:
--
1.7.0.4


2010-07-31 19:18:41

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 2/7] usb: fsl_udc_core: check return value of create_proc_read_entry()

On Sat, Jul 31, 2010 at 09:38:20PM +0400, Kulikov Vasiliy wrote:
> create_proc_read_entry() may fail, if so return -ENOMEM.
>

It can fail, but also we return NULL if procfs is disabled. I haven't
looked at it very carefully, would this patch break the module if procfs
was disabled?

The same applies to the similar patches in this set.

regards,
dan carpenter

2010-08-01 06:19:57

by Kulikov Vasiliy

[permalink] [raw]
Subject: Re: [PATCH 2/7] usb: fsl_udc_core: check return value of create_proc_read_entry()

On Sat, Jul 31, 2010 at 21:17 +0200, Dan Carpenter wrote:
> On Sat, Jul 31, 2010 at 09:38:20PM +0400, Kulikov Vasiliy wrote:
> > create_proc_read_entry() may fail, if so return -ENOMEM.
> >
>
> It can fail, but also we return NULL if procfs is disabled. I haven't
> looked at it very carefully, would this patch break the module if procfs
> was disabled?
Probably you are right, but many drivers in tree compare return value
with NULL. Some of them interpret this as error, some of them simply
call pr_warn("Hmm, I cannot create file in proc, strange..."). Maybe
there is more simplier way to check it without #ifdefs?

>
> The same applies to the similar patches in this set.
>
> regards,
> dan carpenter
>
>

2010-08-01 10:50:10

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 2/7] usb: fsl_udc_core: check return value of create_proc_read_entry()

On Sun, Aug 01, 2010 at 10:19:34AM +0400, Vasiliy Kulikov wrote:
> On Sat, Jul 31, 2010 at 21:17 +0200, Dan Carpenter wrote:
> > On Sat, Jul 31, 2010 at 09:38:20PM +0400, Kulikov Vasiliy wrote:
> > > create_proc_read_entry() may fail, if so return -ENOMEM.
> > >
> >
> > It can fail, but also we return NULL if procfs is disabled. I haven't
> > looked at it very carefully, would this patch break the module if procfs
> > was disabled?
> Probably you are right, but many drivers in tree compare return value
> with NULL. Some of them interpret this as error, some of them simply
> call pr_warn("Hmm, I cannot create file in proc, strange..."). Maybe
> there is more simplier way to check it without #ifdefs?
>

If the allocation fails, there is already a warning so no need to add
another.

These things are one time allocation, normally near boot up when memory
is plentifull. The places that do check should be audited to make sure
there isn't an unneeded dependency on PROC_FS. I would just leave the
rest.

regards,
dan carpenter