On Mon, Jul 27, 2015 at 04:10:29PM +0800, Navy Cheng wrote: > The work flow of dgnc_init_module() is base on if statment. Here is a > nest "if" in the function. > > To avoid the nest if statment, make the code easier to understand and > make the codingstyle to be identical with others, add "end" label to > dgnc_init_module() and use goto to control the work flow. > > Signed-off-by: Navy Cheng <navych@xxxxxxx> > --- Dan Carpenter has explained very beautifully why we should not handle the success and why error values should be handled. You can see his mail at: http://www.spinics.net/lists/linux-driver-devel/msg68202.html Then maybe the following might be a better idea: diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c index 7546aff..b8fa86c 100644 --- a/drivers/staging/dgnc/dgnc_driver.c +++ b/drivers/staging/dgnc/dgnc_driver.c @@ -189,18 +189,20 @@ static int __init dgnc_init_module(void) /* * 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"); + if (rc < 0) + goto err_register_driver; - dgnc_cleanup_module(); - } else { - dgnc_create_driver_sysfiles(&dgnc_driver); - } + dgnc_create_driver_sysfiles(&dgnc_driver); + return 0; + +err_register_driver: + /* 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(); return rc; } Dan ? regards sudip _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel