staging: dgnc: Remove useless and deadly judgment
PATCH 1/1 staging: dgnc: Remove useless and deadly judgment
pci_unregister_driver() should be used once dgnc module exit. It has nothing to do with dgnc_NumBoards. Remove the judgment of dgnc_NumBoards to avoid pci_unregister_driver() is not used when dgnc_NumBoards is 0.
--- drivers/staging/dgnc/dgnc_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/dgnc/dgnc_driver.c +1 -2
@@ -156,8 +156,7 @@ static void dgnc_cleanup_module(void)dgnc_tty_post_uninit (); -if (dgnc_NumBoards) -pci_unregister_driver (&dgnc_driver); +pci_unregister_driver (&dgnc_driver); }/*
Signature · 1 line
2.1.4
PATCH 1/1 staging: dgnc: Remove useless and deadly judgment
drivers/staging/dgnc/dgnc_driver.c static void dgnc_cleanup_module(void) · line 159
Quoted 7 lines · Navy Cheng
pci_unregister_driver()should be used once dgnc module exit. It has nothing to do withdgnc_NumBoards. Remove the judgment ofdgnc_NumBoardsto avoidpci_unregister_driver()is not used whendgnc_NumBoardsis 0.drivers/staging/dgnc/dgnc_driver.c +1
-2-if (dgnc_NumBoards) -pci_unregister_driver (&dgnc_driver); +pci_unregister_driver (&dgnc_driver);
Does pci_unregister_driver do the right thing if there are 0 boards in the system?
If this logic is wrong, shouldn't there also be a patch fixing the following in dgnc_init_module()?
/*
* Find and configure all the cards
*/
rc = pci_register_driver(&dgnc_driver);
/*
* If something went wrong in the scan, bail out of driver.
*/
if (rc < 0) {
/* Only unregister if it was actually registered. */
if (dgnc_NumBoards)
pci_unregister_driver(&dgnc_driver);
else
pr_warn("WARNING: dgnc driver load failed. No Digi Neo or Classic boards found.\n");
dgnc_cleanup_module();While I'm@it, the entire NumBoards counting seems to be wonky - it looks like a *lot* of off-by-one errors. Looks like the programmer(s) weren't sure if they wanted to use that as a zero-based or one-based counter/index.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20160229/4896e710/attachment.bin
PATCH 1/1 staging: dgnc: Remove useless and deadly judgment
Quoted 11 lines · Valdis.Kletnieks at vt.edu
drivers/staging/dgnc/dgnc_driver.c static void dgnc_cleanup_module(void) · line 159
Quoted 7 lines · Navy Cheng
pci_unregister_driver()should be used once dgnc module exit. It has nothing to do withdgnc_NumBoards. Remove the judgment ofdgnc_NumBoardsto avoidpci_unregister_driver()is not used whendgnc_NumBoardsis 0.drivers/staging/dgnc/dgnc_driver.c +1
-2-if (dgnc_NumBoards) -pci_unregister_driver (&dgnc_driver); +pci_unregister_driver (&dgnc_driver);Does
pci_unregister_driverdo the right thing if there are 0 boards in the system?
I have no dgnc device in my laptop. After *insmod* and *rmmod* dgnc module, I can re-insmod dgnc successfully. If dgnc is *rmmod*, the dgnc-file in /sys will disappear.
Quoted 2 lines
If this logic is wrong, shouldn't there also be a patch fixing the following in
dgnc_init_module()?
Yes, I thought dgnc_init_module should be fixed. But I'm not familiar with how to submitting patchset. I decide to fix dgnc_cleanup_module() first.
Quoted 17 lines
/*
* Find and configure all the cards
*/
rc = pci_register_driver(&dgnc_driver);
/*
* If something went wrong in the scan, bail out of driver.
*/
if (rc < 0) {
/* Only unregister if it was actually registered. */
if (dgnc_NumBoards)
pci_unregister_driver(&dgnc_driver);
else
pr_warn("WARNING: dgnc driver load failed. No Digi Neo or Classic boards found.\n");
dgnc_cleanup_module();Quoted 4 lines
While I'm at it, the entire NumBoards counting seems to be wonky - it looks like a *lot* of off-by-one errors. Looks like the programmer(s) weren't sure if they wanted to use that as a zero-based or one-based counter/index.
Maybe we can fix the *init* and *exit* like this:
static int __init dgnc_init_module(void)
{
int rc;
rc = dgnc_start();
if (rc < 0)
goto err1;
rc = pci_register_driver(&dgnc_driver)
if (rc <0) {
goto err1;
pr_warn("....");
}
rc = dgnc_create_driver_sysfiles(&dgnc_driver);
if (rc = 0)
goto err2;
return rc;
err2:
pci_unregister_driver(&dgnc_driver);
err1:
dgnc_end();
return rc;
}
static void dgnc_cleanup_module(void)
{
int i;
dgnc_remove_driver_sysfiles(&dgnc_driver);
for (i = 0; i < dgnc_NumBoards; ++i) {
dgnc_remove_ports_sysfiles(dgnc_Board[i]);
dgnc_tty_uninit(dgnc_Board[i]);
dgnc_cleanup_board(dgnc_Board[i]);
}
pci_unregister_driver(&dgnc_driver);
dgnc_end();
}PATCH 1/1 staging: dgnc: Remove useless and deadly judgment
In reply to Navy ChengQuoted 14 lines · Navy Cheng
pci_unregister_driver()should be used once dgnc module exit. It has nothing to do withdgnc_NumBoards. Remove the judgment ofdgnc_NumBoardsto avoidpci_unregister_driver()is not used whendgnc_NumBoardsis 0.Signed-off-by Navy Cheng <[email protected]>--- drivers/staging/dgnc/dgnc_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/staging/dgnc/dgnc_driver.c +0
-0
drivers/staging/dgnc/dgnc_driver.c static void dgnc_cleanup_module(void) · line 162
Quoted 9 lines
drivers/staging/dgnc/dgnc_driver.c +1
-2@@ -156,8 +156,7 @@ static void dgnc_cleanup_module(void)dgnc_tty_post_uninit (); -if (dgnc_NumBoards) -pci_unregister_driver (&dgnc_driver); +pci_unregister_driver (&dgnc_driver); }/*
Someone else sent a patch that did much this same thing 2 days before you did.
Also please use the get_maintainers.pl file to determine the correct mailing list to send patches like this to (hint, it's not kernelnewbies...)
thanks,
greg k-h