[PATCH 05/05] staging: dgap: Remove printks associated with sysfile creation

[Dan Carpenter]

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 at compro.net>
> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> ---
>  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