linux-kernel - Re: [PATCH 3/3] staging: android: ion_test: unregister the platform device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 2 Apr 2015 01:34:03 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Phong Tran <tranmanphong@...il.com>
Cc:	gregkh@...uxfoundation.org, arve@...roid.com,
	riandrews@...roid.com, devel@...verdev.osuosl.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] staging: android: ion_test: unregister the platform
 device

On Thu, Apr 02, 2015 at 12:38:20AM +0700, Phong Tran wrote:
> The driver has to unregister from platform device when it's unloaded
> 
> Signed-off-by: Phong Tran <tranmanphong@...il.com>
> ---
>  drivers/staging/android/ion/ion_test.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/android/ion/ion_test.c b/drivers/staging/android/ion/ion_test.c
> index f36a35e..d2a236e 100644
> --- a/drivers/staging/android/ion/ion_test.c
> +++ b/drivers/staging/android/ion/ion_test.c
> @@ -278,6 +278,7 @@ static int ion_test_remove(struct platform_device *pdev)
>  	return ret;
>  }
>  
> +static struct platform_device *ion_test_platform_device;

This name is too long.  You will run into the 80 character limit.

>  static struct platform_driver ion_test_platform_driver = {
>  	.remove = ion_test_remove,
>  	.driver = {
> @@ -287,13 +288,21 @@ static struct platform_driver ion_test_platform_driver = {
>  
>  static int __init ion_test_init(void)
>  {
> -	platform_device_register_simple("ion-test", -1, NULL, 0);
> +	ion_test_platform_device = platform_device_register_simple("ion-test",
> +				   -1, NULL, 0);

This indenting is off.  It should be:

	ion_test_dev = platform_device_register_simple("ion-test",
						       -1, NULL, 0);

or something similar.

> +
> +	if (!ion_test_platform_device) {
> +		pr_err("failed to register ion-test platform device\n");

People add error messages without thinking about it because they think,
"Obviously, the more error messages the better."  Almost all the bad
things that can happen in platform_device_register_simple() generate
their own error message.

Also platform_driver_probe() doesn't have an error message so it's
possible to leave them out if you want.  Be bold!  Leave out the
message!  Or if you want go ahead and leave it in.  So long as you have
thought about it is what matters.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/