[PATCH] [POWERPC] remove build warnings in windfarm_core

[PATCH] [POWERPC] remove build warnings in windfarm_core

[Stephen Rothwell]

drivers/macintosh/windfarm_core.c: In function 'wf_register_control':
drivers/macintosh/windfarm_core.c:219: warning: ignoring return value of 'device_create_file', declared with attribute warn_unused_result
drivers/macintosh/windfarm_core.c: In function 'wf_register_sensor':
drivers/macintosh/windfarm_core.c:329: warning: ignoring return value of 'device_create_file', declared with attribute warn_unused_result

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/macintosh/windfarm_core.c |   32 ++++++++++++++++++++++++--------
 1 files changed, 24 insertions(+), 8 deletions(-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --git a/drivers/macintosh/windfarm_core.c b/drivers/macintosh/windfarm_core.c
index 192b26e..a306d67 100644
--- a/drivers/macintosh/windfarm_core.c
+++ b/drivers/macintosh/windfarm_core.c
@@ -198,14 +198,15 @@ static ssize_t wf_store_control(struct device *dev,
 int wf_register_control(struct wf_control *new_ct)
 {
 	struct wf_control *ct;
+	int ret = 0;
 
 	mutex_lock(&wf_lock);
 	list_for_each_entry(ct, &wf_controls, link) {
 		if (!strcmp(ct->name, new_ct->name)) {
 			printk(KERN_WARNING "windfarm: trying to register"
 			       " duplicate control %s\n", ct->name);
-			mutex_unlock(&wf_lock);
-			return -EEXIST;
+			ret = -EEXIST;
+			goto out_unlock;
 		}
 	}
 	kref_init(&new_ct->ref);
@@ -216,14 +217,21 @@ int wf_register_control(struct wf_control *new_ct)
 	new_ct->attr.attr.mode = 0644;
 	new_ct->attr.show = wf_show_control;
 	new_ct->attr.store = wf_store_control;
-	device_create_file(&wf_platform_device.dev, &new_ct->attr);
+	ret = device_create_file(&wf_platform_device.dev, &new_ct->attr);
+	if (ret) {
+		list_del(&new_ct->link);
+		printk(KERN_WARNING "windfarm: device_creat_file failed"
+			"for %s\n", new_ct->name);
+		goto out_unlock;
+	}
 
 	DBG("wf: Registered control %s\n", new_ct->name);
 
 	wf_notify(WF_EVENT_NEW_CONTROL, new_ct);
+out_unlock:
 	mutex_unlock(&wf_lock);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(wf_register_control);
 
@@ -308,14 +316,15 @@ static ssize_t wf_show_sensor(struct device *dev,
 int wf_register_sensor(struct wf_sensor *new_sr)
 {
 	struct wf_sensor *sr;
+	int ret;
 
 	mutex_lock(&wf_lock);
 	list_for_each_entry(sr, &wf_sensors, link) {
 		if (!strcmp(sr->name, new_sr->name)) {
 			printk(KERN_WARNING "windfarm: trying to register"
 			       " duplicate sensor %s\n", sr->name);
-			mutex_unlock(&wf_lock);
-			return -EEXIST;
+			ret = -EEXIST;
+			goto out_unlock;
 		}
 	}
 	kref_init(&new_sr->ref);
@@ -326,14 +335,21 @@ int wf_register_sensor(struct wf_sensor *new_sr)
 	new_sr->attr.attr.mode = 0444;
 	new_sr->attr.show = wf_show_sensor;
 	new_sr->attr.store = NULL;
-	device_create_file(&wf_platform_device.dev, &new_sr->attr);
+	ret = device_create_file(&wf_platform_device.dev, &new_sr->attr);
+	if (ret) {
+		list_del(&new_sr->link);
+		printk(KERN_WARNING "windfarm: device_create_file failed"
+		       " for %s\n", new_sr->name);
+		goto out_unlock;
+	}
 
 	DBG("wf: Registered sensor %s\n", new_sr->name);
 
 	wf_notify(WF_EVENT_NEW_SENSOR, new_sr);
+out_unlock:
 	mutex_unlock(&wf_lock);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(wf_register_sensor);
 
-- 
1.5.1.4

Re: [PATCH] [POWERPC] remove build warnings in windfarm_core

[Paul Mackerras]

Stephen Rothwell writes:

> -	device_create_file(&wf_platform_device.dev, &new_ct->attr);
> +	ret = device_create_file(&wf_platform_device.dev, &new_ct->attr);
> +	if (ret) {
> +		list_del(&new_ct->link);
> +		printk(KERN_WARNING "windfarm: device_creat_file failed"
> +			"for %s\n", new_ct->name);
> +		goto out_unlock;

This shows up why I hate the must_check stuff.  The sysfs files are
not essential for the operation of the windfarm subsystem.  If the
sysfs registration fails for any reason, we now have a completely
non-functional windfarm subsystem instead of a mostly-working one. :(

Paul.

Re: [PATCH] [POWERPC] remove build warnings in windfarm_core

[Segher Boessenkool]

>> -	device_create_file(&wf_platform_device.dev, &new_ct->attr);
>> +	ret = device_create_file(&wf_platform_device.dev, &new_ct->attr);
>> +	if (ret) {
>> +		list_del(&new_ct->link);
>> +		printk(KERN_WARNING "windfarm: device_creat_file failed"
>> +			"for %s\n", new_ct->name);
>> +		goto out_unlock;
>
> This shows up why I hate the must_check stuff.  The sysfs files are
> not essential for the operation of the windfarm subsystem.

They are essential for the user expectations of the
subsystem though; if registration fails, a warning
should be printed.

> If the
> sysfs registration fails for any reason, we now have a completely
> non-functional windfarm subsystem instead of a mostly-working one. :(

Yeah, but that's not must_check's fault, it doesn't
say *what* to do with the error code :-)


Segher

Re: [PATCH] [POWERPC] remove build warnings in windfarm_core

[Linas Vepstas]

On Tue, May 15, 2007 at 08:32:08AM +0200, Segher Boessenkool wrote:
> >> -	device_create_file(&wf_platform_device.dev, &new_ct->attr);
> >> +	ret = device_create_file(&wf_platform_device.dev, &new_ct->attr);
> >> +	if (ret) {
> >> +		list_del(&new_ct->link);
> >> +		printk(KERN_WARNING "windfarm: device_creat_file failed"
> >> +			"for %s\n", new_ct->name);
> >> +		goto out_unlock;
> >
> > This shows up why I hate the must_check stuff.  The sysfs files are
> > not essential for the operation of the windfarm subsystem.
> 
> They are essential for the user expectations of the
> subsystem though; if registration fails, a warning
> should be printed.
> 
> > If the
> > sysfs registration fails for any reason, we now have a completely
> > non-functional windfarm subsystem instead of a mostly-working one. :(
> 
> Yeah, but that's not must_check's fault, it doesn't
> say *what* to do with the error code :-)

Ergo, the patch should be reworked to print the warning only, 
but otherwise continue with the setup.

--linas

[PATCH] [POWERPC] remove build warnings in windfarm_core

[Stephen Rothwell]

drivers/macintosh/windfarm_core.c: In function 'wf_register_control':
drivers/macintosh/windfarm_core.c:219: warning: ignoring return value of 'device_create_file', declared with attribute warn_unused_result
drivers/macintosh/windfarm_core.c: In function 'wf_register_sensor':
drivers/macintosh/windfarm_core.c:329: warning: ignoring return value of 'device_create_file', declared with attribute warn_unused_result

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/macintosh/windfarm_core.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

On Tue, 15 May 2007 16:19:53 +1000 Paul Mackerras <paulus@samba.org> wrote:
>
> This shows up why I hate the must_check stuff.  The sysfs files are
> not essential for the operation of the windfarm subsystem.  If the
> sysfs registration fails for any reason, we now have a completely
> non-functional windfarm subsystem instead of a mostly-working one. :(

This version just warns.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --git a/drivers/macintosh/windfarm_core.c b/drivers/macintosh/windfarm_core.c
index 192b26e..52a95b4 100644
--- a/drivers/macintosh/windfarm_core.c
+++ b/drivers/macintosh/windfarm_core.c
@@ -216,7 +216,10 @@ int wf_register_control(struct wf_control *new_ct)
 	new_ct->attr.attr.mode = 0644;
 	new_ct->attr.show = wf_show_control;
 	new_ct->attr.store = wf_store_control;
-	device_create_file(&wf_platform_device.dev, &new_ct->attr);
+	if (device_create_file(&wf_platform_device.dev, &new_ct->attr))
+		printk(KERN_WARNING "windfarm: device_creat_file failed"
+			"for %s\n", new_ct->name);
+		/* the subsystem still does useful work without the file */
 
 	DBG("wf: Registered control %s\n", new_ct->name);
 
@@ -326,7 +329,10 @@ int wf_register_sensor(struct wf_sensor *new_sr)
 	new_sr->attr.attr.mode = 0444;
 	new_sr->attr.show = wf_show_sensor;
 	new_sr->attr.store = NULL;
-	device_create_file(&wf_platform_device.dev, &new_sr->attr);
+	if (device_create_file(&wf_platform_device.dev, &new_sr->attr))
+		printk(KERN_WARNING "windfarm: device_create_file failed"
+		       " for %s\n", new_sr->name);
+		/* the subsystem still does useful work without the file */
 
 	DBG("wf: Registered sensor %s\n", new_sr->name);
 
-- 
1.5.1.4

Re: [PATCH] [POWERPC] remove build warnings in windfarm_core

[Segher Boessenkool]

> +	if (device_create_file(&wf_platform_device.dev, &new_ct->attr))
> +		printk(KERN_WARNING "windfarm: device_creat_file failed"
> +			"for %s\n", new_ct->name);

Needs a space in the string.


Segher

Re: [PATCH] [POWERPC] remove build warnings in windfarm_core

[Will Schmidt]

On Wed, 2007-16-05 at 11:24 +1000, Stephen Rothwell wrote:

> -	device_create_file(&wf_platform_device.dev, &new_ct->attr);
> +	if (device_create_file(&wf_platform_device.dev, &new_ct->attr))
> +		printk(KERN_WARNING "windfarm: device_creat_file failed"

Hi Stephen, 

s/creat_/create_/


-Will

[PATCH] [POWERPC] remove build warnings in windfarm_core

[Stephen Rothwell]

drivers/macintosh/windfarm_core.c: In function 'wf_register_control':
drivers/macintosh/windfarm_core.c:219: warning: ignoring return value of 'device_create_file', declared with attribute warn_unused_result
drivers/macintosh/windfarm_core.c: In function 'wf_register_sensor':
drivers/macintosh/windfarm_core.c:329: warning: ignoring return value of 'device_create_file', declared with attribute warn_unused_result

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/macintosh/windfarm_core.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

White space and spelling fixes over last version.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --git a/drivers/macintosh/windfarm_core.c b/drivers/macintosh/windfarm_core.c
index 192b26e..11ced17 100644
--- a/drivers/macintosh/windfarm_core.c
+++ b/drivers/macintosh/windfarm_core.c
@@ -216,7 +216,10 @@ int wf_register_control(struct wf_control *new_ct)
 	new_ct->attr.attr.mode = 0644;
 	new_ct->attr.show = wf_show_control;
 	new_ct->attr.store = wf_store_control;
-	device_create_file(&wf_platform_device.dev, &new_ct->attr);
+	if (device_create_file(&wf_platform_device.dev, &new_ct->attr))
+		printk(KERN_WARNING "windfarm: device_create_file failed"
+			" for %s\n", new_ct->name);
+		/* the subsystem still does useful work without the file */
 
 	DBG("wf: Registered control %s\n", new_ct->name);
 
@@ -326,7 +329,10 @@ int wf_register_sensor(struct wf_sensor *new_sr)
 	new_sr->attr.attr.mode = 0444;
 	new_sr->attr.show = wf_show_sensor;
 	new_sr->attr.store = NULL;
-	device_create_file(&wf_platform_device.dev, &new_sr->attr);
+	if (device_create_file(&wf_platform_device.dev, &new_sr->attr))
+		printk(KERN_WARNING "windfarm: device_create_file failed"
+			" for %s\n", new_sr->name);
+		/* the subsystem still does useful work without the file */
 
 	DBG("wf: Registered sensor %s\n", new_sr->name);
 
-- 
1.5.1.4

Re: [PATCH] [POWERPC] remove build warnings in windfarm_core

[Benjamin Herrenschmidt]

On Thu, 2007-05-17 at 11:22 +1000, Stephen Rothwell wrote:
> drivers/macintosh/windfarm_core.c: In function 'wf_register_control':
> drivers/macintosh/windfarm_core.c:219: warning: ignoring return value of 'device_create_file', declared with attribute warn_unused_result
> drivers/macintosh/windfarm_core.c: In function 'wf_register_sensor':
> drivers/macintosh/windfarm_core.c:329: warning: ignoring return value of 'device_create_file', declared with attribute warn_unused_result
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Re: [PATCH] [POWERPC] remove build warnings in windfarm_core

[Geert Uytterhoeven]

On Thu, 17 May 2007, Stephen Rothwell wrote:
> diff --git a/drivers/macintosh/windfarm_core.c b/drivers/macintosh/windfarm_core.c
> index 192b26e..11ced17 100644
> --- a/drivers/macintosh/windfarm_core.c
> +++ b/drivers/macintosh/windfarm_core.c
> @@ -216,7 +216,10 @@ int wf_register_control(struct wf_control *new_ct)
>  	new_ct->attr.attr.mode = 0644;
>  	new_ct->attr.show = wf_show_control;
>  	new_ct->attr.store = wf_store_control;
> -	device_create_file(&wf_platform_device.dev, &new_ct->attr);
> +	if (device_create_file(&wf_platform_device.dev, &new_ct->attr))
> +		printk(KERN_WARNING "windfarm: device_create_file failed"
> +			" for %s\n", new_ct->name);
> +		/* the subsystem still does useful work without the file */

Confusing indentation: the comment is not part of the branch

> @@ -326,7 +329,10 @@ int wf_register_sensor(struct wf_sensor *new_sr)
>  	new_sr->attr.attr.mode = 0444;
>  	new_sr->attr.show = wf_show_sensor;
>  	new_sr->attr.store = NULL;
> -	device_create_file(&wf_platform_device.dev, &new_sr->attr);
> +	if (device_create_file(&wf_platform_device.dev, &new_sr->attr))
> +		printk(KERN_WARNING "windfarm: device_create_file failed"
> +			" for %s\n", new_sr->name);
> +		/* the subsystem still does useful work without the file */

Ditto.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- Sony Network and Software Technology Center Europe (NSCE)
Geert.Uytterhoeven@sonycom.com ------- The Corporate Village, Da Vincilaan 7-D1
Voice +32-2-7008453 Fax +32-2-7008622 ---------------- B-1935 Zaventem, Belgium