[PATCH 7/8] staging: dgap: introduce dgap_cleanup_nodes()

[PATCH 7/8] staging: dgap: introduce dgap_cleanup_nodes()

[Daeseok Youn]

When a configration file is parsed with dgap_parsefile(),
makes nodes for saving configrations for board.

Making a node will allocate node memory and strings for saving
configrations with kstrdup().

So these are freed when dgap is unloaded or failed to initialize.

Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
It MUST be needed to prevent memory leaks but
I'm not sure that I fix properly.

Please review.

 drivers/staging/dgap/dgap.c |   47 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 61baee1..a207bd7 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -200,6 +200,7 @@ static int dgap_test_fep(struct board_t *brd);
 static int dgap_tty_register_ports(struct board_t *brd);
 static int dgap_firmware_load(struct pci_dev *pdev, int card_type,
 			      struct board_t *brd);
+static void dgap_cleanup_nodes(void);
 
 static void dgap_cleanup_module(void);
 
@@ -618,6 +619,7 @@ unregister_tty:
 free_flipbuf:
 	dgap_free_flipbuf(brd);
 cleanup_brd:
+	dgap_cleanup_nodes();
 	dgap_release_remap(brd);
 	kfree(brd);
 
@@ -658,6 +660,8 @@ static void dgap_cleanup_module(void)
 		dgap_cleanup_board(dgap_board[i]);
 	}
 
+	dgap_cleanup_nodes();
+
 	if (dgap_numboards)
 		pci_unregister_driver(&dgap_driver);
 }
@@ -6322,6 +6326,49 @@ static void dgap_remove_tty_sysfs(struct device *c)
 	sysfs_remove_group(&c->kobj, &dgap_tty_attribute_group);
 }
 
+static void dgap_cleanup_nodes(void)
+{
+	struct cnode *p;
+
+	p = &dgap_head;
+
+	while (p) {
+		struct cnode *tmp = p->next;
+
+		switch (p->type) {
+		case BNODE:
+			kfree(p->u.board.portstr);
+			kfree(p->u.board.addrstr);
+			kfree(p->u.board.pcibusstr);
+			kfree(p->u.board.pcislotstr);
+			kfree(p->u.board.method);
+			break;
+		case CNODE:
+			kfree(p->u.conc.id);
+			kfree(p->u.conc.connect);
+			break;
+		case MNODE:
+			kfree(p->u.module.id);
+			break;
+		case TNODE:
+			kfree(p->u.ttyname);
+			break;
+		case CUNODE:
+			kfree(p->u.cuname);
+			break;
+		case LNODE:
+			kfree(p->u.line.cable);
+			break;
+		case PNODE:
+			kfree(p->u.printname);
+			break;
+		}
+
+		kfree(p->u.board.status);
+		kfree(p);
+		p = tmp;
+	}
+}
 /*
  * Parse a configuration file read into memory as a string.
  */
-- 
1.7.1

Re: [PATCH 7/8] staging: dgap: introduce dgap_cleanup_nodes()

[Dan Carpenter]

On Tue, Jul 15, 2014 at 12:04:02PM +0900, Daeseok Youn wrote:
> When a configration file is parsed with dgap_parsefile(),
> makes nodes for saving configrations for board.
> 
> Making a node will allocate node memory and strings for saving
> configrations with kstrdup().
> 
> So these are freed when dgap is unloaded or failed to initialize.
> 
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> ---
> It MUST be needed to prevent memory leaks but
> I'm not sure that I fix properly.
> 
> Please review.

This thread doesn't have Mark on the CC list and he's the only person
who can actually test it properly.  Resend it and CC Mark
Hounschell <markh@compro.net>.

regards,
dan carpenter

Re: [PATCH 7/8] staging: dgap: introduce dgap_cleanup_nodes()

[DaeSeok Youn]

2014-07-15 15:41 GMT+09:00 Dan Carpenter <dan.carpenter@oracle.com>:
> On Tue, Jul 15, 2014 at 12:04:02PM +0900, Daeseok Youn wrote:
>> When a configration file is parsed with dgap_parsefile(),
>> makes nodes for saving configrations for board.
>>
>> Making a node will allocate node memory and strings for saving
>> configrations with kstrdup().
>>
>> So these are freed when dgap is unloaded or failed to initialize.
>>
>> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
>> ---
>> It MUST be needed to prevent memory leaks but
>> I'm not sure that I fix properly.
>>
>> Please review.
>
> This thread doesn't have Mark on the CC list and he's the only person
> who can actually test it properly.  Resend it and CC Mark
> Hounschell <markh@compro.net>.

Really? I'd added Mark to CC list.
hmm.. In your reply doesn't have Mark. I was checking sent-box, he is
in the CC list.

OK. I will try to send this again after fixing 6/7 and 8/8.
Thanks.

regards,
Daeseok Youn
>
> regards,
> dan carpenter
>

Re: [PATCH 7/8] staging: dgap: introduce dgap_cleanup_nodes()

[Dan Carpenter]

On Tue, Jul 15, 2014 at 04:32:27PM +0900, DaeSeok Youn wrote:
> 2014-07-15 15:41 GMT+09:00 Dan Carpenter <dan.carpenter@oracle.com>:
> > On Tue, Jul 15, 2014 at 12:04:02PM +0900, Daeseok Youn wrote:
> >> When a configration file is parsed with dgap_parsefile(),
> >> makes nodes for saving configrations for board.
> >>
> >> Making a node will allocate node memory and strings for saving
> >> configrations with kstrdup().
> >>
> >> So these are freed when dgap is unloaded or failed to initialize.
> >>
> >> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> >> ---
> >> It MUST be needed to prevent memory leaks but
> >> I'm not sure that I fix properly.
> >>
> >> Please review.
> >
> > This thread doesn't have Mark on the CC list and he's the only person
> > who can actually test it properly.  Resend it and CC Mark
> > Hounschell <markh@compro.net>.
> 
> Really? I'd added Mark to CC list.
> hmm.. In your reply doesn't have Mark. I was checking sent-box, he is
> in the CC list.
> 
> OK. I will try to send this again after fixing 6/7 and 8/8.
> Thanks.

None of them have Mark CC'd except for the I added him to the reply
about pr_err().  I knew you said you were CC'ing him so I figured
something weird was going on...

regards,
dan carpenter