Patchwork rio: warn_unused_result warnings fix

login
register
mail settings
Submitter Yang Li
Date April 28, 2009, 10:15 a.m.
Message ID <1240913737-23773-5-git-send-email-leoli@freescale.com>
Download mbox | patch
Permalink /patch/26555/
State Superseded
Delegated to: Kumar Gala
Headers show

Comments

Yang Li - April 28, 2009, 10:15 a.m.
warning: ignoring return value of 'device_add', declared with attribute warn_unused_result
warning: ignoring return value of 'sysfs_create_bin_file', declared with attribute warn_unused_result

Signed-off-by: Li Yang <leoli@freescale.com>
---
 drivers/rapidio/rio-scan.c  |    5 ++++-
 drivers/rapidio/rio-sysfs.c |    6 ++++--
 2 files changed, 8 insertions(+), 3 deletions(-)
Michael Ellerman - April 28, 2009, 10:51 a.m.
On Tue, 2009-04-28 at 18:15 +0800, Li Yang wrote:
> warning: ignoring return value of 'device_add', declared with attribute warn_unused_result
> warning: ignoring return value of 'sysfs_create_bin_file', declared with attribute warn_unused_result
> 
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>  drivers/rapidio/rio-scan.c  |    5 ++++-
>  drivers/rapidio/rio-sysfs.c |    6 ++++--
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rapidio/rio-scan.c b/drivers/rapidio/rio-scan.c
> index 74d0bfa..9309e24 100644
> --- a/drivers/rapidio/rio-scan.c
> +++ b/drivers/rapidio/rio-scan.c
> @@ -265,7 +265,10 @@ static void rio_route_set_ops(struct rio_dev *rdev)
>   */
>  static void __devinit rio_add_device(struct rio_dev *rdev)
>  {
> -	device_add(&rdev->dev);
> +	int err;
> +
> +	err = device_add(&rdev->dev);
> +	WARN_ON(err);
>  
>  	spin_lock(&rio_global_list_lock);
>  	list_add_tail(&rdev->global_list, &rio_devices);

Is that really useful? Why not return the error to rio_setup_device()
which can tell it's caller.

cheers
Yang Li - April 28, 2009, 11:08 a.m.
On Tue, Apr 28, 2009 at 6:51 PM, Michael Ellerman
<michael@ellerman.id.au> wrote:
> On Tue, 2009-04-28 at 18:15 +0800, Li Yang wrote:
>> warning: ignoring return value of 'device_add', declared with attribute warn_unused_result
>> warning: ignoring return value of 'sysfs_create_bin_file', declared with attribute warn_unused_result
>>
>> Signed-off-by: Li Yang <leoli@freescale.com>
>> ---
>>  drivers/rapidio/rio-scan.c  |    5 ++++-
>>  drivers/rapidio/rio-sysfs.c |    6 ++++--
>>  2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/rapidio/rio-scan.c b/drivers/rapidio/rio-scan.c
>> index 74d0bfa..9309e24 100644
>> --- a/drivers/rapidio/rio-scan.c
>> +++ b/drivers/rapidio/rio-scan.c
>> @@ -265,7 +265,10 @@ static void rio_route_set_ops(struct rio_dev *rdev)
>>   */
>>  static void __devinit rio_add_device(struct rio_dev *rdev)
>>  {
>> -     device_add(&rdev->dev);
>> +     int err;
>> +
>> +     err = device_add(&rdev->dev);
>> +     WARN_ON(err);
>>
>>       spin_lock(&rio_global_list_lock);
>>       list_add_tail(&rdev->global_list, &rio_devices);
>
> Is that really useful? Why not return the error to rio_setup_device()
> which can tell it's caller.

IMHO, when device_add() fails the system is quite broken.  So the
value is very limited for it to fail cleanly, which need some effort
to implement.  I can add it if you insist.

- Leo
David Miller - April 28, 2009, 11:38 a.m.
From: Li Yang <leoli@freescale.com>
Date: Tue, 28 Apr 2009 19:08:13 +0800

> IMHO, when device_add() fails the system is quite broken.  So the
> value is very limited for it to fail cleanly, which need some effort
> to implement.  I can add it if you insist.

I disagree.

For the cases where device_add() fails (duplicate name, for
example) the device layer already is emitting warnings.

You're just adding more log messages for the user to sift
through, and likely not adding any new information.

Patch

diff --git a/drivers/rapidio/rio-scan.c b/drivers/rapidio/rio-scan.c
index 74d0bfa..9309e24 100644
--- a/drivers/rapidio/rio-scan.c
+++ b/drivers/rapidio/rio-scan.c
@@ -265,7 +265,10 @@  static void rio_route_set_ops(struct rio_dev *rdev)
  */
 static void __devinit rio_add_device(struct rio_dev *rdev)
 {
-	device_add(&rdev->dev);
+	int err;
+
+	err = device_add(&rdev->dev);
+	WARN_ON(err);
 
 	spin_lock(&rio_global_list_lock);
 	list_add_tail(&rdev->global_list, &rio_devices);
diff --git a/drivers/rapidio/rio-sysfs.c b/drivers/rapidio/rio-sysfs.c
index 97a147f..ba742e8 100644
--- a/drivers/rapidio/rio-sysfs.c
+++ b/drivers/rapidio/rio-sysfs.c
@@ -214,9 +214,11 @@  static struct bin_attribute rio_config_attr = {
  */
 int rio_create_sysfs_dev_files(struct rio_dev *rdev)
 {
-	sysfs_create_bin_file(&rdev->dev.kobj, &rio_config_attr);
+	int err = 0;
 
-	return 0;
+	err = sysfs_create_bin_file(&rdev->dev.kobj, &rio_config_attr);
+
+	return err;
 }
 
 /**