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.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
