2014-07-15 03:05:20

by Daeseok Youn

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

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 <[email protected]>
---
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


2014-07-15 06:41:33

by Dan Carpenter

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

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 <[email protected]>
> ---
> 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 <[email protected]>.

regards,
dan carpenter

2014-07-15 07:32:29

by Daeseok Youn

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

2014-07-15 15:41 GMT+09:00 Dan Carpenter <[email protected]>:
> 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 <[email protected]>
>> ---
>> 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 <[email protected]>.

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
>

2014-07-15 07:57:35

by Dan Carpenter

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

On Tue, Jul 15, 2014 at 04:32:27PM +0900, DaeSeok Youn wrote:
> 2014-07-15 15:41 GMT+09:00 Dan Carpenter <[email protected]>:
> > 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 <[email protected]>
> >> ---
> >> 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 <[email protected]>.
>
> 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