linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: bcm: fix error handling in bcm_init()
@ 2012-09-01 21:37 Alexey Khoroshilov
  2012-09-02 16:51 ` Kevin McKinney
  0 siblings, 1 reply; 6+ messages in thread
From: Alexey Khoroshilov @ 2012-09-01 21:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Alexey Khoroshilov, devel, linux-kernel, ldv-project

bcm_init() does not have proper error handling of usb_register().
The patch implements one.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/staging/bcm/InterfaceInit.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/bcm/InterfaceInit.c b/drivers/staging/bcm/InterfaceInit.c
index 8f85de6..0049890 100644
--- a/drivers/staging/bcm/InterfaceInit.c
+++ b/drivers/staging/bcm/InterfaceInit.c
@@ -669,6 +669,8 @@ struct class *bcm_class;
 
 static __init int bcm_init(void)
 {
+	int retval;
+
 	printk(KERN_INFO "%s: %s, %s\n", DRV_NAME, DRV_DESCRIPTION, DRV_VERSION);
 	printk(KERN_INFO "%s\n", DRV_COPYRIGHT);
 
@@ -678,7 +680,15 @@ static __init int bcm_init(void)
 		return PTR_ERR(bcm_class);
 	}
 
-	return usb_register(&usbbcm_driver);
+	retval = usb_register(&usbbcm_driver);
+	if (retval < 0) {
+		printk(KERN_ERR DRV_NAME ": could not register usb driver\n");
+		goto out_class;
+	}
+	return 0;
+out_class:
+	class_destroy(bcm_class);
+	return retval;
 }
 
 static __exit void bcm_exit(void)
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] staging: bcm: fix error handling in bcm_init()
  2012-09-01 21:37 [PATCH] staging: bcm: fix error handling in bcm_init() Alexey Khoroshilov
@ 2012-09-02 16:51 ` Kevin McKinney
  2012-09-02 19:30   ` [PATCH v2] " Alexey Khoroshilov
  0 siblings, 1 reply; 6+ messages in thread
From: Kevin McKinney @ 2012-09-02 16:51 UTC (permalink / raw)
  To: Alexey Khoroshilov, Greg Kroah-Hartman
  Cc: devel, ldv-project, linux-kernel, klmckinney1, Dan Carpenter

On Sun, Sep 02, 2012 at 01:37:36AM +0400, Alexey Khoroshilov wrote:
> bcm_init() does not have proper error handling of usb_register().
> The patch implements one.
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> ---
>  drivers/staging/bcm/InterfaceInit.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/bcm/InterfaceInit.c b/drivers/staging/bcm/InterfaceInit.c
> index 8f85de6..0049890 100644
> --- a/drivers/staging/bcm/InterfaceInit.c
> +++ b/drivers/staging/bcm/InterfaceInit.c
> @@ -669,6 +669,8 @@ struct class *bcm_class;
>  
>  static __init int bcm_init(void)
>  {
> +	int retval;
> +
>  	printk(KERN_INFO "%s: %s, %s\n", DRV_NAME, DRV_DESCRIPTION, DRV_VERSION);
>  	printk(KERN_INFO "%s\n", DRV_COPYRIGHT);
>  
> @@ -678,7 +680,15 @@ static __init int bcm_init(void)
>  		return PTR_ERR(bcm_class);
>  	}
>  
> -	return usb_register(&usbbcm_driver);
> +	retval = usb_register(&usbbcm_driver);
> +	if (retval < 0) {
> +		printk(KERN_ERR DRV_NAME ": could not register usb driver\n");
> +		goto out_class;
> +	}
> +	return 0;
> +out_class:
> +	class_destroy(bcm_class);
> +	return retval;
>  }
>  
Hi Alexey,

Instead of using a goto statement, can we just: (1) destory the class and (2) return retval within the if statement? i.e:

if (retval < 0) {
	printk(KERN_ERR DRV_NAME ": could not register usb driver\n");
	+class_destroy(bcm_class);
	+return retval;
}

goto statements are very useful and handy when a function exits from multiple locations because they provide a 
common exit point. The compiler will translate this into a "jump" instruction. However in this case I am not sure
it is simplifying our logic because we only have one usage. lets just call the two lines of code directly within the
if statement.

In addition, while you are in this code can you replace the "printk" statements with the preferred "pr_info" in a separate patch?  

Thanks,
Kevin

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2] staging: bcm: fix error handling in bcm_init()
  2012-09-02 16:51 ` Kevin McKinney
@ 2012-09-02 19:30   ` Alexey Khoroshilov
  2012-09-02 19:30     ` [PATCH] staging: bcm: use pr_info and pr_err rather than printk Alexey Khoroshilov
  2012-09-02 20:38     ` [PATCH v2] staging: bcm: fix error handling in bcm_init() Kevin McKinney
  0 siblings, 2 replies; 6+ messages in thread
From: Alexey Khoroshilov @ 2012-09-02 19:30 UTC (permalink / raw)
  To: Kevin McKinney, Greg Kroah-Hartman
  Cc: Alexey Khoroshilov, Dan Carpenter, devel, linux-kernel, ldv-project

bcm_init() does not have proper error handling of usb_register().
The patch implements one.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/staging/bcm/InterfaceInit.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/bcm/InterfaceInit.c b/drivers/staging/bcm/InterfaceInit.c
index 8f85de6..57452ef 100644
--- a/drivers/staging/bcm/InterfaceInit.c
+++ b/drivers/staging/bcm/InterfaceInit.c
@@ -669,6 +669,8 @@ struct class *bcm_class;
 
 static __init int bcm_init(void)
 {
+	int retval;
+
 	printk(KERN_INFO "%s: %s, %s\n", DRV_NAME, DRV_DESCRIPTION, DRV_VERSION);
 	printk(KERN_INFO "%s\n", DRV_COPYRIGHT);
 
@@ -678,7 +680,13 @@ static __init int bcm_init(void)
 		return PTR_ERR(bcm_class);
 	}
 
-	return usb_register(&usbbcm_driver);
+	retval = usb_register(&usbbcm_driver);
+	if (retval < 0) {
+		printk(KERN_ERR DRV_NAME ": could not register usb driver\n");
+		class_destroy(bcm_class);
+		return retval;
+	}
+	return 0;
 }
 
 static __exit void bcm_exit(void)
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH] staging: bcm: use pr_info and pr_err rather than printk
  2012-09-02 19:30   ` [PATCH v2] " Alexey Khoroshilov
@ 2012-09-02 19:30     ` Alexey Khoroshilov
  2012-09-02 20:42       ` Kevin McKinney
  2012-09-02 20:38     ` [PATCH v2] staging: bcm: fix error handling in bcm_init() Kevin McKinney
  1 sibling, 1 reply; 6+ messages in thread
From: Alexey Khoroshilov @ 2012-09-02 19:30 UTC (permalink / raw)
  To: Kevin McKinney, Greg Kroah-Hartman
  Cc: Alexey Khoroshilov, Dan Carpenter, devel, linux-kernel, ldv-project

Convert printk(KERN_INFO to pr_info( and printk(KERN_ERR to pr_err(.

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/staging/bcm/InterfaceInit.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/bcm/InterfaceInit.c b/drivers/staging/bcm/InterfaceInit.c
index 57452ef..dfee0ce 100644
--- a/drivers/staging/bcm/InterfaceInit.c
+++ b/drivers/staging/bcm/InterfaceInit.c
@@ -671,18 +671,18 @@ static __init int bcm_init(void)
 {
 	int retval;
 
-	printk(KERN_INFO "%s: %s, %s\n", DRV_NAME, DRV_DESCRIPTION, DRV_VERSION);
-	printk(KERN_INFO "%s\n", DRV_COPYRIGHT);
+	pr_info("%s: %s, %s\n", DRV_NAME, DRV_DESCRIPTION, DRV_VERSION);
+	pr_info("%s\n", DRV_COPYRIGHT);
 
 	bcm_class = class_create(THIS_MODULE, DRV_NAME);
 	if (IS_ERR(bcm_class)) {
-		printk(KERN_ERR DRV_NAME ": could not create class\n");
+		pr_err(DRV_NAME ": could not create class\n");
 		return PTR_ERR(bcm_class);
 	}
 
 	retval = usb_register(&usbbcm_driver);
 	if (retval < 0) {
-		printk(KERN_ERR DRV_NAME ": could not register usb driver\n");
+		pr_err(DRV_NAME ": could not register usb driver\n");
 		class_destroy(bcm_class);
 		return retval;
 	}
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] staging: bcm: fix error handling in bcm_init()
  2012-09-02 19:30   ` [PATCH v2] " Alexey Khoroshilov
  2012-09-02 19:30     ` [PATCH] staging: bcm: use pr_info and pr_err rather than printk Alexey Khoroshilov
@ 2012-09-02 20:38     ` Kevin McKinney
  1 sibling, 0 replies; 6+ messages in thread
From: Kevin McKinney @ 2012-09-02 20:38 UTC (permalink / raw)
  To: Alexey Khoroshilov, Greg Kroah-Hartman
  Cc: Dan Carpenter, devel, linux-kernel, ldv-project

On Sun, Sep 02, 2012 at 11:30:13PM +0400, Alexey Khoroshilov wrote:
> bcm_init() does not have proper error handling of usb_register().
> The patch implements one.
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> ---
>  drivers/staging/bcm/InterfaceInit.c |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/bcm/InterfaceInit.c b/drivers/staging/bcm/InterfaceInit.c
> index 8f85de6..57452ef 100644
> --- a/drivers/staging/bcm/InterfaceInit.c
> +++ b/drivers/staging/bcm/InterfaceInit.c
> @@ -669,6 +669,8 @@ struct class *bcm_class;
>  
Acked-by: Kevin McKinney <klmckinney1@gmail.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] staging: bcm: use pr_info and pr_err rather than printk
  2012-09-02 19:30     ` [PATCH] staging: bcm: use pr_info and pr_err rather than printk Alexey Khoroshilov
@ 2012-09-02 20:42       ` Kevin McKinney
  0 siblings, 0 replies; 6+ messages in thread
From: Kevin McKinney @ 2012-09-02 20:42 UTC (permalink / raw)
  To: Alexey Khoroshilov, Greg Kroah-Hartman
  Cc: Dan Carpenter, devel, linux-kernel, ldv-project, klmckinney1

On Sun, Sep 02, 2012 at 11:30:14PM +0400, Alexey Khoroshilov wrote:
> Convert printk(KERN_INFO to pr_info( and printk(KERN_ERR to pr_err(.
> 
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> ---
>  drivers/staging/bcm/InterfaceInit.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/bcm/InterfaceInit.c b/drivers/staging/bcm/InterfaceInit.c
> index 57452ef..dfee0ce 100644
> --- a/drivers/staging/bcm/InterfaceInit.c
> +++ b/drivers/staging/bcm/InterfaceInit.c
> @@ -671,18 +671,18 @@ static __init int bcm_init(void)

Acked-by: Kevin McKinney <klmckinney1@gmail.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-02 20:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-01 21:37 [PATCH] staging: bcm: fix error handling in bcm_init() Alexey Khoroshilov
2012-09-02 16:51 ` Kevin McKinney
2012-09-02 19:30   ` [PATCH v2] " Alexey Khoroshilov
2012-09-02 19:30     ` [PATCH] staging: bcm: use pr_info and pr_err rather than printk Alexey Khoroshilov
2012-09-02 20:42       ` Kevin McKinney
2012-09-02 20:38     ` [PATCH v2] staging: bcm: fix error handling in bcm_init() Kevin McKinney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).