Re: [PATCH] scsi: ses fix mem leaking when fail to add intf

2008-02-10 Thread Yinghai Lu
On Sunday 10 February 2008 08:28:38 pm James Bottomley wrote:
> 
> On Sat, 2008-02-09 at 15:15 -0800, Yinghai Lu wrote:
> > [PATCH] scsi: ses fix mem leaking when fail to add intf
> > 
> > fix leaking with scomp leaking when failing.
> > also remove one extra space.
> 
> There are still a few extraneous code moves in this one.  This is about
> the correct minimal set, isn't it?


if buf allocation for page 7 get NULL...

if put 
+ if (!buf)
+   goto err_free;

still not right, because still undo 
edev = enclosure_register(cdev->dev, sdev->sdev_gendev.bus_id,
  components, &ses_enclosure_callbacks);

all just add 
+ if (!buf)
+   goto simple_populate;

there?

YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] scsi: ses fix mem leaking when fail to add intf

2008-02-10 Thread James Bottomley

On Sat, 2008-02-09 at 15:15 -0800, Yinghai Lu wrote:
> [PATCH] scsi: ses fix mem leaking when fail to add intf
> 
> fix leaking with scomp leaking when failing.
> also remove one extra space.

There are still a few extraneous code moves in this one.  This is about
the correct minimal set, isn't it?

James

---

From: Yinghai Lu <[EMAIL PROTECTED]>
Date: Sat, 9 Feb 2008 15:15:47 -0800
Subject: [SCSI] ses: fix memory leaks

fix leaking with scomp leaking when failing. Also free page10 on
driver removal  and remove one extra space.

Signed-off-by: Yinghai Lu <[EMAIL PROTECTED]>
Signed-off-by: James Bottomley <[EMAIL PROTECTED]>
---
 drivers/scsi/ses.c |   20 
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 2a6e4f4..8abc4a9 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -416,11 +416,11 @@ static int ses_intf_add(struct class_device *cdev,
int i, j, types, len, components = 0;
int err = -ENOMEM;
struct enclosure_device *edev;
-   struct ses_component *scomp;
+   struct ses_component *scomp = NULL;
 
if (!scsi_device_enclosure(sdev)) {
/* not an enclosure, but might be in one */
-   edev =  enclosure_find(&sdev->host->shost_gendev);
+   edev = enclosure_find(&sdev->host->shost_gendev);
if (edev) {
ses_match_to_enclosure(edev, sdev);
class_device_put(&edev->cdev);
@@ -456,9 +456,6 @@ static int ses_intf_add(struct class_device *cdev,
if (!buf)
goto err_free;
 
-   ses_dev->page1 = buf;
-   ses_dev->page1_len = len;
-
result = ses_recv_diag(sdev, 1, buf, len);
if (result)
goto recv_failed;
@@ -473,6 +470,9 @@ static int ses_intf_add(struct class_device *cdev,
type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE)
components += type_ptr[1];
}
+   ses_dev->page1 = buf;
+   ses_dev->page1_len = len;
+   buf = NULL;
 
result = ses_recv_diag(sdev, 2, hdr_buf, INIT_ALLOC_SIZE);
if (result)
@@ -489,6 +489,7 @@ static int ses_intf_add(struct class_device *cdev,
goto recv_failed;
ses_dev->page2 = buf;
ses_dev->page2_len = len;
+   buf = NULL;
 
/* The additional information page --- allows us
 * to match up the devices */
@@ -506,11 +507,12 @@ static int ses_intf_add(struct class_device *cdev,
goto recv_failed;
ses_dev->page10 = buf;
ses_dev->page10_len = len;
+   buf = NULL;
 
  no_page10:
-   scomp = kmalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
+   scomp = kzalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
if (!scomp)
-   goto  err_free;
+   goto err_free;
 
edev = enclosure_register(cdev->dev, sdev->sdev_gendev.bus_id,
  components, &ses_enclosure_callbacks);
@@ -521,7 +523,7 @@ static int ses_intf_add(struct class_device *cdev,
 
edev->scratch = ses_dev;
for (i = 0; i < components; i++)
-   edev->component[i].scratch = scomp++;
+   edev->component[i].scratch = scomp + i;
 
/* Page 7 for the descriptors is optional */
buf = NULL;
@@ -598,6 +600,7 @@ static int ses_intf_add(struct class_device *cdev,
err = -ENODEV;
  err_free:
kfree(buf);
+   kfree(scomp);
kfree(ses_dev->page10);
kfree(ses_dev->page2);
kfree(ses_dev->page1);
@@ -630,6 +633,7 @@ static void ses_intf_remove(struct class_device *cdev,
ses_dev = edev->scratch;
edev->scratch = NULL;
 
+   kfree(ses_dev->page10);
kfree(ses_dev->page1);
kfree(ses_dev->page2);
kfree(ses_dev);
-- 
1.5.3.8



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] scsi: ses fix mem leaking when fail to add intf

2008-02-09 Thread Yinghai Lu
[PATCH] scsi: ses fix mem leaking when fail to add intf

fix leaking with scomp leaking when failing.
also remove one extra space.

Signed-off-by: Yinghai Lu <[EMAIL PROTECTED]>

Index: linux-2.6/drivers/scsi/ses.c
===
--- linux-2.6.orig/drivers/scsi/ses.c
+++ linux-2.6/drivers/scsi/ses.c
@@ -416,11 +416,11 @@ static int ses_intf_add(struct class_dev
int i, j, types, len, components = 0;
int err = -ENOMEM;
struct enclosure_device *edev;
-   struct ses_component *scomp;
+   struct ses_component *scomp = NULL;
 
if (!scsi_device_enclosure(sdev)) {
/* not an enclosure, but might be in one */
-   edev =  enclosure_find(&sdev->host->shost_gendev);
+   edev = enclosure_find(&sdev->host->shost_gendev);
if (edev) {
ses_match_to_enclosure(edev, sdev);
class_device_put(&edev->cdev);
@@ -456,9 +456,6 @@ static int ses_intf_add(struct class_dev
if (!buf)
goto err_free;
 
-   ses_dev->page1 = buf;
-   ses_dev->page1_len = len;
-
result = ses_recv_diag(sdev, 1, buf, len);
if (result)
goto recv_failed;
@@ -473,6 +470,9 @@ static int ses_intf_add(struct class_dev
type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE)
components += type_ptr[1];
}
+   ses_dev->page1 = buf;
+   ses_dev->page1_len = len;
+   buf = NULL;
 
result = ses_recv_diag(sdev, 2, hdr_buf, INIT_ALLOC_SIZE);
if (result)
@@ -489,6 +489,7 @@ static int ses_intf_add(struct class_dev
goto recv_failed;
ses_dev->page2 = buf;
ses_dev->page2_len = len;
+   buf = NULL;
 
/* The additional information page --- allows us
 * to match up the devices */
@@ -506,11 +507,26 @@ static int ses_intf_add(struct class_dev
goto recv_failed;
ses_dev->page10 = buf;
ses_dev->page10_len = len;
+   buf = NULL;
 
  no_page10:
-   scomp = kmalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
+
+   /* Page 7 for the descriptors is optional */
+   result = ses_recv_diag(sdev, 7, hdr_buf, INIT_ALLOC_SIZE);
+   if (result)
+   goto simple_populate;
+
+   len = (hdr_buf[2] << 8) + hdr_buf[3] + 4;
+   /* add 1 for trailing '\0' we'll use */
+   buf = kzalloc(len + 1, GFP_KERNEL);
+   if (!buf)
+   goto err_free;
+   result = ses_recv_diag(sdev, 7, buf, len);
+
+ simple_populate:
+   scomp = kzalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
if (!scomp)
-   goto  err_free;
+   goto err_free;
 
edev = enclosure_register(cdev->dev, sdev->sdev_gendev.bus_id,
  components, &ses_enclosure_callbacks);
@@ -521,20 +537,10 @@ static int ses_intf_add(struct class_dev
 
edev->scratch = ses_dev;
for (i = 0; i < components; i++)
-   edev->component[i].scratch = scomp++;
+   edev->component[i].scratch = scomp + i;
 
-   /* Page 7 for the descriptors is optional */
-   buf = NULL;
-   result = ses_recv_diag(sdev, 7, hdr_buf, INIT_ALLOC_SIZE);
-   if (result)
-   goto simple_populate;
-
-   len = (hdr_buf[2] << 8) + hdr_buf[3] + 4;
-   /* add 1 for trailing '\0' we'll use */
-   buf = kzalloc(len + 1, GFP_KERNEL);
-   result = ses_recv_diag(sdev, 7, buf, len);
+   /* result and buf from page 7 check */
if (result) {
- simple_populate:
kfree(buf);
buf = NULL;
desc_ptr = NULL;
@@ -598,6 +604,7 @@ static int ses_intf_add(struct class_dev
err = -ENODEV;
  err_free:
kfree(buf);
+   kfree(scomp);
kfree(ses_dev->page10);
kfree(ses_dev->page2);
kfree(ses_dev->page1);
@@ -630,6 +637,7 @@ static void ses_intf_remove(struct class
ses_dev = edev->scratch;
edev->scratch = NULL;
 
+   kfree(ses_dev->page10);
kfree(ses_dev->page1);
kfree(ses_dev->page2);
kfree(ses_dev);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/