All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
@ 2011-08-22 14:00 ` Julia Lawall
  0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-22 14:00 UTC (permalink / raw)
  To: Robert Love
  Cc: kernel-janitors, James E.J. Bottomley, devel, linux-scsi, linux-kernel

From: Julia Lawall <julia@diku.dk>

The initializations of both fcoe_nport_scsi_transport and
fcoe_vport_scsi_transport can fail, so test both of them.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r@
identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
statement S;
@@

x = f(...);
(
if (\(x == NULL\|IS_ERR(x)\)) S
|
*if (\(y == NULL\|IS_ERR(y)\))
 { ... when != x
   return ...; }
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/scsi/fcoe/fcoe.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index ba710e3..921b636 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1096,7 +1096,7 @@ static int __init fcoe_if_init(void)
 	fcoe_vport_scsi_transport =
 		fc_attach_transport(&fcoe_vport_fc_functions);
 
-	if (!fcoe_nport_scsi_transport) {
+	if (!fcoe_nport_scsi_transport || !fcoe_vport_scsi_transport) {
 		printk(KERN_ERR "fcoe: Failed to attach to the FC transport\n");
 		return -ENODEV;
 	}


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
@ 2011-08-22 14:00 ` Julia Lawall
  0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-22 14:00 UTC (permalink / raw)
  To: Robert Love
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	devel-s9riP+hp16TNLxjTenLetw, James E.J. Bottomley,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA

From: Julia Lawall <julia@diku.dk>

The initializations of both fcoe_nport_scsi_transport and
fcoe_vport_scsi_transport can fail, so test both of them.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r@
identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
statement S;
@@

x = f(...);
(
if (\(x = NULL\|IS_ERR(x)\)) S
|
*if (\(y = NULL\|IS_ERR(y)\))
 { ... when != x
   return ...; }
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/scsi/fcoe/fcoe.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index ba710e3..921b636 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1096,7 +1096,7 @@ static int __init fcoe_if_init(void)
 	fcoe_vport_scsi_transport  		fc_attach_transport(&fcoe_vport_fc_functions);
 
-	if (!fcoe_nport_scsi_transport) {
+	if (!fcoe_nport_scsi_transport || !fcoe_vport_scsi_transport) {
 		printk(KERN_ERR "fcoe: Failed to attach to the FC transport\n");
 		return -ENODEV;
 	}


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
@ 2011-08-22 14:00 ` Julia Lawall
  0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-22 14:00 UTC (permalink / raw)
  To: Robert Love
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	devel-s9riP+hp16TNLxjTenLetw, James E.J. Bottomley,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA

From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>

The initializations of both fcoe_nport_scsi_transport and
fcoe_vport_scsi_transport can fail, so test both of them.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r@
identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
statement S;
@@

x = f(...);
(
if (\(x == NULL\|IS_ERR(x)\)) S
|
*if (\(y == NULL\|IS_ERR(y)\))
 { ... when != x
   return ...; }
)
// </smpl>

Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>

---
 drivers/scsi/fcoe/fcoe.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index ba710e3..921b636 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1096,7 +1096,7 @@ static int __init fcoe_if_init(void)
 	fcoe_vport_scsi_transport =
 		fc_attach_transport(&fcoe_vport_fc_functions);
 
-	if (!fcoe_nport_scsi_transport) {
+	if (!fcoe_nport_scsi_transport || !fcoe_vport_scsi_transport) {
 		printk(KERN_ERR "fcoe: Failed to attach to the FC transport\n");
 		return -ENODEV;
 	}

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
  2011-08-22 14:00 ` Julia Lawall
@ 2011-08-22 14:34   ` Jesper Juhl
  -1 siblings, 0 replies; 15+ messages in thread
From: Jesper Juhl @ 2011-08-22 14:34 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Robert Love, kernel-janitors, James E.J. Bottomley, devel,
	linux-scsi, linux-kernel

On Mon, 22 Aug 2011, Julia Lawall wrote:

> From: Julia Lawall <julia@diku.dk>
> 
> The initializations of both fcoe_nport_scsi_transport and
> fcoe_vport_scsi_transport can fail, so test both of them.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r@
> identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
> statement S;
> @@
> 
> x = f(...);
> (
> if (\(x == NULL\|IS_ERR(x)\)) S
> |
> *if (\(y == NULL\|IS_ERR(y)\))
>  { ... when != x
>    return ...; }
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  drivers/scsi/fcoe/fcoe.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index ba710e3..921b636 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -1096,7 +1096,7 @@ static int __init fcoe_if_init(void)
>  	fcoe_vport_scsi_transport =
>  		fc_attach_transport(&fcoe_vport_fc_functions);
>  
> -	if (!fcoe_nport_scsi_transport) {
> +	if (!fcoe_nport_scsi_transport || !fcoe_vport_scsi_transport) {
>  		printk(KERN_ERR "fcoe: Failed to attach to the FC transport\n");
>  		return -ENODEV;
>  	}
> 

I only took a quick look, so I may have overlooked something, so bear with 
me.

fc_attach_transport() allocates memory with kzalloc. If either call fails 
the other may have succeeded and we'll leak the memory allocated to one of 
them.

Shouldn't we be kfree()'ing the two variables before the 'return -ENODEV'?

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
@ 2011-08-22 14:34   ` Jesper Juhl
  0 siblings, 0 replies; 15+ messages in thread
From: Jesper Juhl @ 2011-08-22 14:34 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Robert Love, kernel-janitors, James E.J. Bottomley, devel,
	linux-scsi, linux-kernel

On Mon, 22 Aug 2011, Julia Lawall wrote:

> From: Julia Lawall <julia@diku.dk>
> 
> The initializations of both fcoe_nport_scsi_transport and
> fcoe_vport_scsi_transport can fail, so test both of them.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r@
> identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
> statement S;
> @@
> 
> x = f(...);
> (
> if (\(x = NULL\|IS_ERR(x)\)) S
> |
> *if (\(y = NULL\|IS_ERR(y)\))
>  { ... when != x
>    return ...; }
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  drivers/scsi/fcoe/fcoe.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index ba710e3..921b636 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -1096,7 +1096,7 @@ static int __init fcoe_if_init(void)
>  	fcoe_vport_scsi_transport >  		fc_attach_transport(&fcoe_vport_fc_functions);
>  
> -	if (!fcoe_nport_scsi_transport) {
> +	if (!fcoe_nport_scsi_transport || !fcoe_vport_scsi_transport) {
>  		printk(KERN_ERR "fcoe: Failed to attach to the FC transport\n");
>  		return -ENODEV;
>  	}
> 

I only took a quick look, so I may have overlooked something, so bear with 
me.

fc_attach_transport() allocates memory with kzalloc. If either call fails 
the other may have succeeded and we'll leak the memory allocated to one of 
them.

Shouldn't we be kfree()'ing the two variables before the 'return -ENODEV'?

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
  2011-08-22 14:34   ` Jesper Juhl
@ 2011-08-22 14:36     ` Julia Lawall
  -1 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-22 14:36 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: Robert Love, kernel-janitors, James E.J. Bottomley, devel,
	linux-scsi, linux-kernel

On Mon, 22 Aug 2011, Jesper Juhl wrote:

> On Mon, 22 Aug 2011, Julia Lawall wrote:
> 
> > From: Julia Lawall <julia@diku.dk>
> > 
> > The initializations of both fcoe_nport_scsi_transport and
> > fcoe_vport_scsi_transport can fail, so test both of them.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @r@
> > identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
> > statement S;
> > @@
> > 
> > x = f(...);
> > (
> > if (\(x == NULL\|IS_ERR(x)\)) S
> > |
> > *if (\(y == NULL\|IS_ERR(y)\))
> >  { ... when != x
> >    return ...; }
> > )
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> >  drivers/scsi/fcoe/fcoe.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> > index ba710e3..921b636 100644
> > --- a/drivers/scsi/fcoe/fcoe.c
> > +++ b/drivers/scsi/fcoe/fcoe.c
> > @@ -1096,7 +1096,7 @@ static int __init fcoe_if_init(void)
> >  	fcoe_vport_scsi_transport =
> >  		fc_attach_transport(&fcoe_vport_fc_functions);
> >  
> > -	if (!fcoe_nport_scsi_transport) {
> > +	if (!fcoe_nport_scsi_transport || !fcoe_vport_scsi_transport) {
> >  		printk(KERN_ERR "fcoe: Failed to attach to the FC transport\n");
> >  		return -ENODEV;
> >  	}
> > 
> 
> I only took a quick look, so I may have overlooked something, so bear with 
> me.
> 
> fc_attach_transport() allocates memory with kzalloc. If either call fails 
> the other may have succeeded and we'll leak the memory allocated to one of 
> them.
> 
> Shouldn't we be kfree()'ing the two variables before the 'return -ENODEV'?

It seems reasonable.

julia

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
@ 2011-08-22 14:36     ` Julia Lawall
  0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-22 14:36 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: Robert Love, kernel-janitors, James E.J. Bottomley, devel,
	linux-scsi, linux-kernel

On Mon, 22 Aug 2011, Jesper Juhl wrote:

> On Mon, 22 Aug 2011, Julia Lawall wrote:
> 
> > From: Julia Lawall <julia@diku.dk>
> > 
> > The initializations of both fcoe_nport_scsi_transport and
> > fcoe_vport_scsi_transport can fail, so test both of them.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @r@
> > identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
> > statement S;
> > @@
> > 
> > x = f(...);
> > (
> > if (\(x = NULL\|IS_ERR(x)\)) S
> > |
> > *if (\(y = NULL\|IS_ERR(y)\))
> >  { ... when != x
> >    return ...; }
> > )
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> >  drivers/scsi/fcoe/fcoe.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> > index ba710e3..921b636 100644
> > --- a/drivers/scsi/fcoe/fcoe.c
> > +++ b/drivers/scsi/fcoe/fcoe.c
> > @@ -1096,7 +1096,7 @@ static int __init fcoe_if_init(void)
> >  	fcoe_vport_scsi_transport > >  		fc_attach_transport(&fcoe_vport_fc_functions);
> >  
> > -	if (!fcoe_nport_scsi_transport) {
> > +	if (!fcoe_nport_scsi_transport || !fcoe_vport_scsi_transport) {
> >  		printk(KERN_ERR "fcoe: Failed to attach to the FC transport\n");
> >  		return -ENODEV;
> >  	}
> > 
> 
> I only took a quick look, so I may have overlooked something, so bear with 
> me.
> 
> fc_attach_transport() allocates memory with kzalloc. If either call fails 
> the other may have succeeded and we'll leak the memory allocated to one of 
> them.
> 
> Shouldn't we be kfree()'ing the two variables before the 'return -ENODEV'?

It seems reasonable.

julia

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
  2011-08-22 14:34   ` Jesper Juhl
@ 2011-08-22 14:49     ` Julia Lawall
  -1 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-22 14:49 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: Robert Love, kernel-janitors, James E.J. Bottomley, devel,
	linux-scsi, linux-kernel

>From nobody Mon Aug 22 15:57:42 CEST 2011
From: Julia Lawall <julia@diku.dk>
To: Robert Love <robert.w.love@intel.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>,devel@open-fcoe.org,linux-scsi@vger.kernel.org,linux-kernel@vger.kernel.org
Subject: [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test

From: Julia Lawall <julia@diku.dk>

The initializations of both fcoe_nport_scsi_transport and
fcoe_vport_scsi_transport can fail, so test both of them.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r@
identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
statement S;
@@

x = f(...);
(
if (\(x == NULL\|IS_ERR(x)\)) S
|
*if (\(y == NULL\|IS_ERR(y)\))
 { ... when != x
   return ...; }
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/scsi/fcoe/fcoe.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index ba710e3..9f2e2f4 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1093,11 +1093,20 @@ static int __init fcoe_if_init(void)
 	/* attach to scsi transport */
 	fcoe_nport_scsi_transport =
 		fc_attach_transport(&fcoe_nport_fc_functions);
+
+	if (!fcoe_nport_scsi_transport) {
+		printk(KERN_ERR
+		       "fcoe_nport: Failed to attach to the FC transport\n");
+		return -ENODEV;
+	}
+
 	fcoe_vport_scsi_transport =
 		fc_attach_transport(&fcoe_vport_fc_functions);
 
-	if (!fcoe_nport_scsi_transport) {
-		printk(KERN_ERR "fcoe: Failed to attach to the FC transport\n");
+	if (!fcoe_vport_scsi_transport) {
+		printk(KERN_ERR
+		       "fcoe_vport: Failed to attach to the FC transport\n");
+		fc_release_transport(fcoe_nport_scsi_transport);
 		return -ENODEV;
 	}
 

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
@ 2011-08-22 14:49     ` Julia Lawall
  0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-22 14:49 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: Robert Love, kernel-janitors, James E.J. Bottomley, devel,
	linux-scsi, linux-kernel

From nobody Mon Aug 22 15:57:42 CEST 2011
From: Julia Lawall <julia@diku.dk>
To: Robert Love <robert.w.love@intel.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>,devel@open-fcoe.org,linux-scsi@vger.kernel.org,linux-kernel@vger.kernel.org
Subject: [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test

From: Julia Lawall <julia@diku.dk>

The initializations of both fcoe_nport_scsi_transport and
fcoe_vport_scsi_transport can fail, so test both of them.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r@
identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
statement S;
@@

x = f(...);
(
if (\(x = NULL\|IS_ERR(x)\)) S
|
*if (\(y = NULL\|IS_ERR(y)\))
 { ... when != x
   return ...; }
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/scsi/fcoe/fcoe.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index ba710e3..9f2e2f4 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1093,11 +1093,20 @@ static int __init fcoe_if_init(void)
 	/* attach to scsi transport */
 	fcoe_nport_scsi_transport  		fc_attach_transport(&fcoe_nport_fc_functions);
+
+	if (!fcoe_nport_scsi_transport) {
+		printk(KERN_ERR
+		       "fcoe_nport: Failed to attach to the FC transport\n");
+		return -ENODEV;
+	}
+
 	fcoe_vport_scsi_transport  		fc_attach_transport(&fcoe_vport_fc_functions);
 
-	if (!fcoe_nport_scsi_transport) {
-		printk(KERN_ERR "fcoe: Failed to attach to the FC transport\n");
+	if (!fcoe_vport_scsi_transport) {
+		printk(KERN_ERR
+		       "fcoe_vport: Failed to attach to the FC transport\n");
+		fc_release_transport(fcoe_nport_scsi_transport);
 		return -ENODEV;
 	}
 

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* RE: [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
  2011-08-22 14:49     ` Julia Lawall
@ 2011-08-23  7:23       ` Zou, Yi
  -1 siblings, 0 replies; 15+ messages in thread
From: Zou, Yi @ 2011-08-23  7:23 UTC (permalink / raw)
  To: Julia Lawall, Jesper Juhl
  Cc: linux-scsi, kernel-janitors, linux-kernel, James E.J. Bottomley, devel

> From nobody Mon Aug 22 15:57:42 CEST 2011
> From: Julia Lawall <julia@diku.dk>
> To: Robert Love <robert.w.love@intel.com>
> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>,devel@open-
> fcoe.org,linux-scsi@vger.kernel.org,linux-kernel@vger.kernel.org
> Subject: [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
> 
> From: Julia Lawall <julia@diku.dk>
> 
> The initializations of both fcoe_nport_scsi_transport and
> fcoe_vport_scsi_transport can fail, so test both of them.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r@
> identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
> statement S;
> @@
> 
> x = f(...);
> (
> if (\(x == NULL\|IS_ERR(x)\)) S
> |
> *if (\(y == NULL\|IS_ERR(y)\))
>  { ... when != x
>    return ...; }
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  drivers/scsi/fcoe/fcoe.c |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index ba710e3..9f2e2f4 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -1093,11 +1093,20 @@ static int __init fcoe_if_init(void)
>  	/* attach to scsi transport */
>  	fcoe_nport_scsi_transport =
>  		fc_attach_transport(&fcoe_nport_fc_functions);
> +
> +	if (!fcoe_nport_scsi_transport) {
> +		printk(KERN_ERR
> +		       "fcoe_nport: Failed to attach to the FC transport\n");
> +		return -ENODEV;
> +	}
> +
>  	fcoe_vport_scsi_transport =
>  		fc_attach_transport(&fcoe_vport_fc_functions);
> 
> -	if (!fcoe_nport_scsi_transport) {
> -		printk(KERN_ERR "fcoe: Failed to attach to the FC
> transport\n");
> +	if (!fcoe_vport_scsi_transport) {
> +		printk(KERN_ERR
> +		       "fcoe_vport: Failed to attach to the FC transport\n");
> +		fc_release_transport(fcoe_nport_scsi_transport);
>  		return -ENODEV;
>  	}
> 

I understand we were using -ENODEV before, but -ENOMEM is preferred here 
as that's the only err from fc_attach_transport.

Thanks,
yi


^ permalink raw reply	[flat|nested] 15+ messages in thread
* RE: [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing
@ 2011-08-23  7:23       ` Zou, Yi
  0 siblings, 0 replies; 15+ messages in thread
From: Zou, Yi @ 2011-08-23  7:23 UTC (permalink / raw)
  To: Julia Lawall, Jesper Juhl
  Cc: linux-scsi, kernel-janitors, linux-kernel, James E.J. Bottomley, devel

> From nobody Mon Aug 22 15:57:42 CEST 2011
> From: Julia Lawall <julia@diku.dk>
> To: Robert Love <robert.w.love@intel.com>
> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>,devel@open-
> fcoe.org,linux-scsi@vger.kernel.org,linux-kernel@vger.kernel.org
> Subject: [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
> 
> From: Julia Lawall <julia@diku.dk>
> 
> The initializations of both fcoe_nport_scsi_transport and
> fcoe_vport_scsi_transport can fail, so test both of them.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r@
> identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
> statement S;
> @@
> 
> x = f(...);
> (
> if (\(x = NULL\|IS_ERR(x)\)) S
> |
> *if (\(y = NULL\|IS_ERR(y)\))
>  { ... when != x
>    return ...; }
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
>  drivers/scsi/fcoe/fcoe.c |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index ba710e3..9f2e2f4 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -1093,11 +1093,20 @@ static int __init fcoe_if_init(void)
>  	/* attach to scsi transport */
>  	fcoe_nport_scsi_transport >  		fc_attach_transport(&fcoe_nport_fc_functions);
> +
> +	if (!fcoe_nport_scsi_transport) {
> +		printk(KERN_ERR
> +		       "fcoe_nport: Failed to attach to the FC transport\n");
> +		return -ENODEV;
> +	}
> +
>  	fcoe_vport_scsi_transport >  		fc_attach_transport(&fcoe_vport_fc_functions);
> 
> -	if (!fcoe_nport_scsi_transport) {
> -		printk(KERN_ERR "fcoe: Failed to attach to the FC
> transport\n");
> +	if (!fcoe_vport_scsi_transport) {
> +		printk(KERN_ERR
> +		       "fcoe_vport: Failed to attach to the FC transport\n");
> +		fc_release_transport(fcoe_nport_scsi_transport);
>  		return -ENODEV;
>  	}
> 

I understand we were using -ENODEV before, but -ENOMEM is preferred here 
as that's the only err from fc_attach_transport.

Thanks,
yi


^ permalink raw reply	[flat|nested] 15+ messages in thread
* RE: [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
  2011-08-23  7:23       ` [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing Zou, Yi
@ 2011-08-23  8:33         ` Julia Lawall
  -1 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-23  8:33 UTC (permalink / raw)
  To: Zou, Yi
  Cc: Jesper Juhl, linux-scsi, kernel-janitors, linux-kernel,
	James E.J. Bottomley, devel

On Tue, 23 Aug 2011, Zou, Yi wrote:

> > From nobody Mon Aug 22 15:57:42 CEST 2011
> > From: Julia Lawall <julia@diku.dk>
> > To: Robert Love <robert.w.love@intel.com>
> > Cc: "James E.J. Bottomley" <JBottomley@parallels.com>,devel@open-
> > fcoe.org,linux-scsi@vger.kernel.org,linux-kernel@vger.kernel.org
> > Subject: [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
> > 
> > From: Julia Lawall <julia@diku.dk>
> > 
> > The initializations of both fcoe_nport_scsi_transport and
> > fcoe_vport_scsi_transport can fail, so test both of them.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @r@
> > identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
> > statement S;
> > @@
> > 
> > x = f(...);
> > (
> > if (\(x == NULL\|IS_ERR(x)\)) S
> > |
> > *if (\(y == NULL\|IS_ERR(y)\))
> >  { ... when != x
> >    return ...; }
> > )
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> >  drivers/scsi/fcoe/fcoe.c |   13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> > index ba710e3..9f2e2f4 100644
> > --- a/drivers/scsi/fcoe/fcoe.c
> > +++ b/drivers/scsi/fcoe/fcoe.c
> > @@ -1093,11 +1093,20 @@ static int __init fcoe_if_init(void)
> >  	/* attach to scsi transport */
> >  	fcoe_nport_scsi_transport =
> >  		fc_attach_transport(&fcoe_nport_fc_functions);
> > +
> > +	if (!fcoe_nport_scsi_transport) {
> > +		printk(KERN_ERR
> > +		       "fcoe_nport: Failed to attach to the FC transport\n");
> > +		return -ENODEV;
> > +	}
> > +
> >  	fcoe_vport_scsi_transport =
> >  		fc_attach_transport(&fcoe_vport_fc_functions);
> > 
> > -	if (!fcoe_nport_scsi_transport) {
> > -		printk(KERN_ERR "fcoe: Failed to attach to the FC
> > transport\n");
> > +	if (!fcoe_vport_scsi_transport) {
> > +		printk(KERN_ERR
> > +		       "fcoe_vport: Failed to attach to the FC transport\n");
> > +		fc_release_transport(fcoe_nport_scsi_transport);
> >  		return -ENODEV;
> >  	}
> > 
> 
> I understand we were using -ENODEV before, but -ENOMEM is preferred here 
> as that's the only err from fc_attach_transport.

OK, I'll fix it.  Thanks.

julia

^ permalink raw reply	[flat|nested] 15+ messages in thread
* RE: [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing
@ 2011-08-23  8:33         ` Julia Lawall
  0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-23  8:33 UTC (permalink / raw)
  To: Zou, Yi
  Cc: Jesper Juhl, linux-scsi, kernel-janitors, linux-kernel,
	James E.J. Bottomley, devel

On Tue, 23 Aug 2011, Zou, Yi wrote:

> > From nobody Mon Aug 22 15:57:42 CEST 2011
> > From: Julia Lawall <julia@diku.dk>
> > To: Robert Love <robert.w.love@intel.com>
> > Cc: "James E.J. Bottomley" <JBottomley@parallels.com>,devel@open-
> > fcoe.org,linux-scsi@vger.kernel.org,linux-kernel@vger.kernel.org
> > Subject: [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
> > 
> > From: Julia Lawall <julia@diku.dk>
> > 
> > The initializations of both fcoe_nport_scsi_transport and
> > fcoe_vport_scsi_transport can fail, so test both of them.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @r@
> > identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
> > statement S;
> > @@
> > 
> > x = f(...);
> > (
> > if (\(x = NULL\|IS_ERR(x)\)) S
> > |
> > *if (\(y = NULL\|IS_ERR(y)\))
> >  { ... when != x
> >    return ...; }
> > )
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> >  drivers/scsi/fcoe/fcoe.c |   13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> > index ba710e3..9f2e2f4 100644
> > --- a/drivers/scsi/fcoe/fcoe.c
> > +++ b/drivers/scsi/fcoe/fcoe.c
> > @@ -1093,11 +1093,20 @@ static int __init fcoe_if_init(void)
> >  	/* attach to scsi transport */
> >  	fcoe_nport_scsi_transport > >  		fc_attach_transport(&fcoe_nport_fc_functions);
> > +
> > +	if (!fcoe_nport_scsi_transport) {
> > +		printk(KERN_ERR
> > +		       "fcoe_nport: Failed to attach to the FC transport\n");
> > +		return -ENODEV;
> > +	}
> > +
> >  	fcoe_vport_scsi_transport > >  		fc_attach_transport(&fcoe_vport_fc_functions);
> > 
> > -	if (!fcoe_nport_scsi_transport) {
> > -		printk(KERN_ERR "fcoe: Failed to attach to the FC
> > transport\n");
> > +	if (!fcoe_vport_scsi_transport) {
> > +		printk(KERN_ERR
> > +		       "fcoe_vport: Failed to attach to the FC transport\n");
> > +		fc_release_transport(fcoe_nport_scsi_transport);
> >  		return -ENODEV;
> >  	}
> > 
> 
> I understand we were using -ENODEV before, but -ENOMEM is preferred here 
> as that's the only err from fc_attach_transport.

OK, I'll fix it.  Thanks.

julia

^ permalink raw reply	[flat|nested] 15+ messages in thread
* RE: [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test
  2011-08-23  7:23       ` [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing Zou, Yi
@ 2011-08-23  8:42         ` Julia Lawall
  -1 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-23  8:42 UTC (permalink / raw)
  To: Zou, Yi
  Cc: Jesper Juhl, linux-scsi, kernel-janitors, linux-kernel,
	James E.J. Bottomley, devel

From: Julia Lawall <julia@diku.dk>

The initializations of both fcoe_nport_scsi_transport and
fcoe_vport_scsi_transport can fail, so test both of them.

Convert the return value to -ENOMEM to better reflect the reason for the 
error.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r@
identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
statement S;
@@

x = f(...);
(
if (\(x == NULL\|IS_ERR(x)\)) S
|
*if (\(y == NULL\|IS_ERR(y)\))
 { ... when != x
   return ...; }
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/scsi/fcoe/fcoe.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index ba710e3..2a279eb 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1093,12 +1093,21 @@ static int __init fcoe_if_init(void)
 	/* attach to scsi transport */
 	fcoe_nport_scsi_transport =
 		fc_attach_transport(&fcoe_nport_fc_functions);
+
+	if (!fcoe_nport_scsi_transport) {
+		printk(KERN_ERR
+		       "fcoe_nport: Failed to attach to the FC transport\n");
+		return -ENOMEM;
+	}
+
 	fcoe_vport_scsi_transport =
 		fc_attach_transport(&fcoe_vport_fc_functions);
 
-	if (!fcoe_nport_scsi_transport) {
-		printk(KERN_ERR "fcoe: Failed to attach to the FC transport\n");
-		return -ENODEV;
+	if (!fcoe_vport_scsi_transport) {
+		printk(KERN_ERR
+		       "fcoe_vport: Failed to attach to the FC transport\n");
+		fc_release_transport(fcoe_nport_scsi_transport);
+		return -ENOMEM;
 	}
 
 	return 0;

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* RE: [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing
@ 2011-08-23  8:42         ` Julia Lawall
  0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-23  8:42 UTC (permalink / raw)
  To: Zou, Yi
  Cc: Jesper Juhl, linux-scsi, kernel-janitors, linux-kernel,
	James E.J. Bottomley, devel

From: Julia Lawall <julia@diku.dk>

The initializations of both fcoe_nport_scsi_transport and
fcoe_vport_scsi_transport can fail, so test both of them.

Convert the return value to -ENOMEM to better reflect the reason for the 
error.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r@
identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
statement S;
@@

x = f(...);
(
if (\(x = NULL\|IS_ERR(x)\)) S
|
*if (\(y = NULL\|IS_ERR(y)\))
 { ... when != x
   return ...; }
)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/scsi/fcoe/fcoe.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index ba710e3..2a279eb 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1093,12 +1093,21 @@ static int __init fcoe_if_init(void)
 	/* attach to scsi transport */
 	fcoe_nport_scsi_transport  		fc_attach_transport(&fcoe_nport_fc_functions);
+
+	if (!fcoe_nport_scsi_transport) {
+		printk(KERN_ERR
+		       "fcoe_nport: Failed to attach to the FC transport\n");
+		return -ENOMEM;
+	}
+
 	fcoe_vport_scsi_transport  		fc_attach_transport(&fcoe_vport_fc_functions);
 
-	if (!fcoe_nport_scsi_transport) {
-		printk(KERN_ERR "fcoe: Failed to attach to the FC transport\n");
-		return -ENODEV;
+	if (!fcoe_vport_scsi_transport) {
+		printk(KERN_ERR
+		       "fcoe_vport: Failed to attach to the FC transport\n");
+		fc_release_transport(fcoe_nport_scsi_transport);
+		return -ENOMEM;
 	}
 
 	return 0;

^ permalink raw reply related	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-08-23  8:42 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-22 14:00 [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test Julia Lawall
2011-08-22 14:00 ` Julia Lawall
2011-08-22 14:00 ` Julia Lawall
2011-08-22 14:34 ` Jesper Juhl
2011-08-22 14:34   ` Jesper Juhl
2011-08-22 14:36   ` Julia Lawall
2011-08-22 14:36     ` Julia Lawall
2011-08-22 14:49   ` Julia Lawall
2011-08-22 14:49     ` Julia Lawall
2011-08-23  7:23     ` [Open-FCoE] " Zou, Yi
2011-08-23  7:23       ` [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing Zou, Yi
2011-08-23  8:33       ` [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test Julia Lawall
2011-08-23  8:33         ` [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing Julia Lawall
2011-08-23  8:42       ` [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing test Julia Lawall
2011-08-23  8:42         ` [Open-FCoE] [PATCH 2/4] drivers/scsi/fcoe/fcoe.c: add missing Julia Lawall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.