* [PATCH 7/8 RESEND] staging: dgap: introduce dgap_cleanup_nodes()
@ 2014-07-15 9:14 Daeseok Youn
2014-07-15 15:30 ` Greg KH
[not found] ` <790027403.325559.1405438239093.JavaMail.root@mx2.compro.net>
0 siblings, 2 replies; 5+ messages in thread
From: Daeseok Youn @ 2014-07-15 9:14 UTC (permalink / raw)
To: lidza.louina, markh
Cc: markh, daeseok.youn, gregkh, driverdev-devel, devel, linux-kernel
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>
---
RESEND: Adds Mark to TO list and CC list for
checking send this email properly to him.
And also he can verify this patch.
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 9e750fb..e4ea68b 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -202,6 +202,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);
@@ -620,6 +621,7 @@ unregister_tty:
free_flipbuf:
dgap_free_flipbuf(brd);
cleanup_brd:
+ dgap_cleanup_nodes();
dgap_release_remap(brd);
kfree(brd);
@@ -660,6 +662,8 @@ static void dgap_cleanup_module(void)
dgap_cleanup_board(dgap_board[i]);
}
+ dgap_cleanup_nodes();
+
if (dgap_numboards)
pci_unregister_driver(&dgap_driver);
}
@@ -6324,6 +6328,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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 7/8 RESEND] staging: dgap: introduce dgap_cleanup_nodes()
2014-07-15 9:14 [PATCH 7/8 RESEND] staging: dgap: introduce dgap_cleanup_nodes() Daeseok Youn
@ 2014-07-15 15:30 ` Greg KH
[not found] ` <790027403.325559.1405438239093.JavaMail.root@mx2.compro.net>
1 sibling, 0 replies; 5+ messages in thread
From: Greg KH @ 2014-07-15 15:30 UTC (permalink / raw)
To: Daeseok Youn; +Cc: lidza.louina, markh, devel, driverdev-devel, linux-kernel
On Tue, Jul 15, 2014 at 06:14:25PM +0900, Daeseok Youn wrote:
> When a configration file is parsed with dgap_parsefile(),
> makes nodes for saving configrations for board.
configuration files should not be parsed in the kernel at all. That
logic should be removed as it should not be needed.
Mark, can you verify that this is not needed with your hardware anymore?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 7/8 RESEND] staging: dgap: introduce dgap_cleanup_nodes()
[not found] ` <790027403.325559.1405438239093.JavaMail.root@mx2.compro.net>
@ 2014-07-16 12:39 ` Mark Hounschell
2014-07-16 13:17 ` Dan Carpenter
2014-07-16 23:59 ` DaeSeok Youn
0 siblings, 2 replies; 5+ messages in thread
From: Mark Hounschell @ 2014-07-16 12:39 UTC (permalink / raw)
To: Greg KH, Daeseok Youn; +Cc: lidza.louina, devel, driverdev-devel, linux-kernel
On 07/15/2014 11:30 AM, Greg KH wrote:
> On Tue, Jul 15, 2014 at 06:14:25PM +0900, Daeseok Youn wrote:
>> When a configration file is parsed with dgap_parsefile(),
>> makes nodes for saving configrations for board.
>
> configuration files should not be parsed in the kernel at all. That
> logic should be removed as it should not be needed.
>
> Mark, can you verify that this is not needed with your hardware anymore?
>
As far as I can see it is still needed. If no config file, no devices
are created and configured. I had planned to remove that code such that
all the different cards supported are still supported. Even though I
only have 2 different types of cards. I Just haven't got to it yet. I
realize "configuration files should not be parsed in the kernel" and I'm
certainly not going to argue that. But the way it is, it allows user
land to define board order and even device node names (additional links
to the actual device). No matter where boards are installed and found by
the kernel, you have persistent naming. I guess all that can be done
with udev (well, sort of anyway).
There are also options in the config file that should be passed in as
module parameters
and I planned to do that part before messing with the config file code.
We also have this "DIGI_EXPANDERS_SUPPORTED" thingy. I have none of
these cards. These cards interrogate the expander to find out how many
ports there are. Then configure accordingly. Should we just remove all
that code and NOT support expanders since we don't currently have the
hardware to test?
I will update from your current git tree and test this week. I've been
swamped lately and unable to do much. I thought I'd be available to do
some more work but things come up as you know.
Regards
Mark
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 7/8 RESEND] staging: dgap: introduce dgap_cleanup_nodes()
2014-07-16 12:39 ` Mark Hounschell
@ 2014-07-16 13:17 ` Dan Carpenter
2014-07-16 23:59 ` DaeSeok Youn
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2014-07-16 13:17 UTC (permalink / raw)
To: Mark Hounschell
Cc: Greg KH, Daeseok Youn, devel, lidza.louina, driverdev-devel,
linux-kernel
On Wed, Jul 16, 2014 at 08:39:33AM -0400, Mark Hounschell wrote:
> We also have this "DIGI_EXPANDERS_SUPPORTED" thingy. I have none of
> these cards. These cards interrogate the expander to find out how
> many ports there are. Then configure accordingly. Should we just
> remove all that code and NOT support expanders since we don't
> currently have the hardware to test?
Maybe eventually, but it's not like we have run out of other less
painfull cleanups that we can do right away.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 7/8 RESEND] staging: dgap: introduce dgap_cleanup_nodes()
2014-07-16 12:39 ` Mark Hounschell
2014-07-16 13:17 ` Dan Carpenter
@ 2014-07-16 23:59 ` DaeSeok Youn
1 sibling, 0 replies; 5+ messages in thread
From: DaeSeok Youn @ 2014-07-16 23:59 UTC (permalink / raw)
To: Mark Hounschell
Cc: Greg KH, Lidza Louina, devel, driverdev-devel, linux-kernel,
Dan Carpenter
Hi, Mark
Thanks for reply.
2014-07-16 21:39 GMT+09:00 Mark Hounschell <markh@compro.net>:
> On 07/15/2014 11:30 AM, Greg KH wrote:
>>
>> On Tue, Jul 15, 2014 at 06:14:25PM +0900, Daeseok Youn wrote:
>>>
>>> When a configration file is parsed with dgap_parsefile(),
>>> makes nodes for saving configrations for board.
>>
>>
>> configuration files should not be parsed in the kernel at all. That
>> logic should be removed as it should not be needed.
>>
>> Mark, can you verify that this is not needed with your hardware anymore?
>>
>
> As far as I can see it is still needed. If no config file, no devices are
> created and configured. I had planned to remove that code such that all the
> different cards supported are still supported. Even though I only have 2
> different types of cards. I Just haven't got to it yet. I realize
> "configuration files should not be parsed in the kernel" and I'm certainly
> not going to argue that. But the way it is, it allows user land to define
> board order and even device node names (additional links to the actual
> device). No matter where boards are installed and found by the kernel, you
> have persistent naming. I guess all that can be done with udev (well, sort
> of anyway).
>
> There are also options in the config file that should be passed in as module
> parameters
> and I planned to do that part before messing with the config file code.
>
> We also have this "DIGI_EXPANDERS_SUPPORTED" thingy. I have none of these
> cards. These cards interrogate the expander to find out how many ports there
> are. Then configure accordingly. Should we just remove all that code and NOT
> support expanders since we don't currently have the hardware to test?
>
> I will update from your current git tree and test this week. I've been
> swamped lately and unable to do much. I thought I'd be available to do some
> more work but things come up as you know.
It need to have this patch when you start to test dgap module. This patch may be
cleaned up memory which are allocated in dgap_parse() when dgap fails
to init or unloads.
And I will send this patch again.
Thanks.
Daeseok Youn.
>
> Regards
> Mark
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-07-16 23:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-15 9:14 [PATCH 7/8 RESEND] staging: dgap: introduce dgap_cleanup_nodes() Daeseok Youn
2014-07-15 15:30 ` Greg KH
[not found] ` <790027403.325559.1405438239093.JavaMail.root@mx2.compro.net>
2014-07-16 12:39 ` Mark Hounschell
2014-07-16 13:17 ` Dan Carpenter
2014-07-16 23:59 ` DaeSeok Youn
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).