On 03/06/2014 02:29 PM, Dan Carpenter wrote:
On Thu, Mar 06, 2014 at 01:57:55PM -0500, Mark Hounschell wrote:This patch removes printks associated with sysfile creation and changes the dgap_create_driver_sysfiles function to return an int so we can check for errors in the sysfile creation process. The printk's were flagged by checkpatch but then driver_create_file was flagged by checkpatch for not checking its return. So we remove the printk's and check the return of driver_create_file. Signed-off-by: Mark Hounschell <markh@xxxxxxxxxx> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- drivers/staging/dgap/dgap.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c index 9e4afa1..3e7cb18 100644 --- a/drivers/staging/dgap/dgap.c +++ b/drivers/staging/dgap/dgap.c @@ -193,7 +193,7 @@ struct class_device; static void dgap_create_ports_sysfiles(struct board_t *bd); static void dgap_remove_ports_sysfiles(struct board_t *bd); -static void dgap_create_driver_sysfiles(struct pci_driver *); +static int dgap_create_driver_sysfiles(struct pci_driver *); static void dgap_remove_driver_sysfiles(struct pci_driver *); static void dgap_create_tty_sysfs(struct un_t *un, struct device *c); @@ -544,8 +544,11 @@ static int dgap_init_module(void) dgap_cleanup_module(); } else { - dgap_create_driver_sysfiles(&dgap_driver); - dgap_driver_state = DRIVER_READY; + rc = dgap_create_driver_sysfiles(&dgap_driver); + if (rc) + dgap_cleanup_module(); + else + dgap_driver_state = DRIVER_READY;There is a bug a couple lines earlier there we call pci_unregister_driver(&dgap_driver) but it's also called in dgap_cleanup_module(). Double free. Really this error handling is more complicated than it needs to be. rc = dgap_start(); if (rc) return rc; rc = dgap_init_pci(); if (rc) goto err_cleanup; rc = dgap_create_driver_sysfiles(&dgap_driver); if (rc) goto err_cleanup; dgap_driver_state = DRIVER_READY; return 0; err_cleanup: dgap_cleanup_module(); return rc; } I removed most of the comments because the comments belong with the function implementation and not with the callers. In my version the success path is just a straight line of function calls at level one indent.
want me to do this is in a separate patch or redo this one? Mark _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
