[PATCH] Convert compile time warnings into runtime warnings.

[Christian Gmeiner-4]

HI all,

this patch fixes following compile time warnings:

/root/em8300/modules/em8300_i2c.c: In function 'em8300_i2c_reg':
/root/em8300/modules/em8300_i2c.c:116: warning: ignoring return value
of 'sysfs_create_link', declared with attribute warn_unused_result
/root/em8300/modules/em8300_i2c.c:124: warning: ignoring return value
of 'sysfs_create_link', declared with attribute warn_unused_result
/root/em8300/modules/em8300_i2c.c:127: warning: ignoring return value
of 'sysfs_create_link', declared with attribute warn_unused_result

greets,
Christian Gmeiner


-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Nicolas Boullis]

Hi Christian,

On Tue, Dec 04, 2007 at 12:49:10AM +0100, Christian Gmeiner wrote:

> HI all,
>
> this patch fixes following compile time warnings:
>
> /root/em8300/modules/em8300_i2c.c: In function 'em8300_i2c_reg':
> /root/em8300/modules/em8300_i2c.c:116: warning: ignoring return value
> of 'sysfs_create_link', declared with attribute warn_unused_result
> /root/em8300/modules/em8300_i2c.c:124: warning: ignoring return value
> of 'sysfs_create_link', declared with attribute warn_unused_result
> /root/em8300/modules/em8300_i2c.c:127: warning: ignoring return value
> of 'sysfs_create_link', declared with attribute warn_unused_result

I have no objection to a runtime warning, but do you think it's worth
failing the regitration of the i2c client only because the symlink
creation failed?


Nicolas

-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Christian Gmeiner-4]

2007/12/4, Nicolas Boullis <nboullis@...>:

> Hi Christian,
>
> On Tue, Dec 04, 2007 at 12:49:10AM +0100, Christian Gmeiner wrote:
> > HI all,
> >
> > this patch fixes following compile time warnings:
> >
> > /root/em8300/modules/em8300_i2c.c: In function 'em8300_i2c_reg':
> > /root/em8300/modules/em8300_i2c.c:116: warning: ignoring return value
> > of 'sysfs_create_link', declared with attribute warn_unused_result
> > /root/em8300/modules/em8300_i2c.c:124: warning: ignoring return value
> > of 'sysfs_create_link', declared with attribute warn_unused_result
> > /root/em8300/modules/em8300_i2c.c:127: warning: ignoring return value
> > of 'sysfs_create_link', declared with attribute warn_unused_result
>
> I have no objection to a runtime warning, but do you think it's worth
> failing the regitration of the i2c client only because the symlink
> creation failed?

Good point... I have here an corrected patch may you have a look at it.

Greets,
Christian

> Nicolas
>
> -------------------------------------------------------------------------
> SF.Net email is sponsored by: The Future of Linux Business White Paper
> from Novell.  From the desktop to the data center, Linux is going
> mainstream.  Let it simplify your IT future.
> http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
> _______________________________________________
> Dxr3-devel mailing list
> Dxr3-devel@...
> https://lists.sourceforge.net/lists/listinfo/dxr3-devel
>

-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Nicolas Boullis]

Hi,

On Wed, Dec 05, 2007 at 08:35:46AM +0100, Christian Gmeiner wrote:
> >
> > I have no objection to a runtime warning, but do you think it's worth
> > failing the regitration of the i2c client only because the symlink
> > creation failed?
>
> Good point... I have here an corrected patch may you have a look at it.

I think it is not correct yet. Firstly because err is uninitialized in
case of an unexpected client. Secondly because you did nof fix my
sysfs_create_link macro for old kernels.


Cheers,

Nicolas

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Christian Gmeiner-4]

2007/12/6, Nicolas Boullis <nboullis@...>:

> Hi,
>
> On Wed, Dec 05, 2007 at 08:35:46AM +0100, Christian Gmeiner wrote:
> > >
> > > I have no objection to a runtime warning, but do you think it's worth
> > > failing the regitration of the i2c client only because the symlink
> > > creation failed?
> >
> > Good point... I have here an corrected patch may you have a look at it.
>
> I think it is not correct yet. Firstly because err is uninitialized in
> case of an unexpected client. Secondly because you did nof fix my
> sysfs_create_link macro for old kernels.

Hi Nicolas,

you are correct once more... here is a fixed version.

Greets,
Christian


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Nicolas Boullis]

Hi Christian,

On Fri, Dec 07, 2007 at 12:34:33AM +0100, Christian Gmeiner wrote:
>
> Hi Nicolas,
>
> you are correct once more... here is a fixed version.

I think it is functionally correct, but I don't love the way it is
organized. It certainly is a matter of taste, but I'd rather use "err"
for errors that are to be propagated outside the current function.
Moreover, I don't think this symlink creation should be propagated
anyway, even outside the switch block.

How about the attached patch (to avoid code duplication), or perhaps
even better the use of a separate function?


Cheers,

Nicolas

---

Index: em8300_i2c.c
===================================================================
RCS file: /cvsroot/dxr3/em8300/modules/em8300_i2c.c,v
retrieving revision 1.32
diff -u -r1.32 em8300_i2c.c
--- em8300_i2c.c 9 Nov 2007 22:11:39 -0000 1.32
+++ em8300_i2c.c 7 Dec 2007 22:45:53 -0000
@@ -26,7 +26,7 @@
 //#include <linux/sensors.h>
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)
-#define sysfs_create_link(kobj, target, name) do {} while (0)
+#define sysfs_create_link(kobj, target, name) 0
 #define sysfs_remove_link(kobj, name) do {} while (0)
 #else
 #include <linux/sysfs.h>
@@ -100,6 +100,7 @@
 static int em8300_i2c_reg(struct i2c_client *client)
 {
  struct em8300_s *em = i2c_get_adapdata(client->adapter);
+ char *link_name;
 
  switch (client->driver->id) {
  case I2C_DRIVERID_ADV717X:
@@ -113,18 +114,22 @@
  em->encoder_type = ENCODER_ADV7170;
  }
  em->encoder = client;
- sysfs_create_link(&em->dev->dev.kobj, &client->dev.kobj, "encoder");
- break;
+ link_name = "encoder";
+ goto create_link;
  case I2C_DRIVERID_BT865:
  if (em8300_i2c_lock_client(client)) {
  return -ENODEV;
  }
  em->encoder_type = ENCODER_BT865;
  em->encoder = client;
- sysfs_create_link(&em->dev->dev.kobj, &client->dev.kobj, "encoder");
- break;
+ link_name = "encoder";
+ goto create_link;
  case I2C_DRIVERID_EEPROM:
- sysfs_create_link(&em->dev->dev.kobj, &client->dev.kobj, "eeprom");
+ link_name = "eeprom";
+ goto create_link;
+ create_link:
+ if (sysfs_create_link(&em->dev->dev.kobj, &client->dev.kobj, link_name))
+ printk(KERN_WARNING "em8300_i2c: unable to create the %s link\n", link_name);
  break;
  default:
  printk(KERN_ERR "em8300_i2c: unknown client id\n");

---

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Nicolas Boullis]

Hi,

On Fri, Dec 07, 2007 at 11:47:56PM +0100, Nicolas Boullis wrote:

> Hi Christian,
>
> On Fri, Dec 07, 2007 at 12:34:33AM +0100, Christian Gmeiner wrote:
> >
> > Hi Nicolas,
> >
> > you are correct once more... here is a fixed version.
>
> I think it is functionally correct, but I don't love the way it is
> organized. It certainly is a matter of taste, but I'd rather use "err"
> for errors that are to be propagated outside the current function.
> Moreover, I don't think this symlink creation should be propagated
> anyway, even outside the switch block.
>
> How about the attached patch (to avoid code duplication), or perhaps
> even better the use of a separate function?

That patch is not compatible with the latest changes; so I considered
writing a small macro to avoid code duplication.

Any opinion about this new patch?


Nicolas

---

? modules/Module.symvers
Index: modules/em8300_i2c.c
===================================================================
RCS file: /cvsroot/dxr3/em8300/modules/em8300_i2c.c,v
retrieving revision 1.34
diff -u -r1.34 em8300_i2c.c
--- modules/em8300_i2c.c 2 Jan 2008 23:00:40 -0000 1.34
+++ modules/em8300_i2c.c 5 Jan 2008 23:07:44 -0000
@@ -27,7 +27,7 @@
 //#include <linux/sensors.h>
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)
-#define sysfs_create_link(kobj, target, name) do {} while (0)
+#define sysfs_create_link(kobj, target, name) 0
 #define sysfs_remove_link(kobj, name) do {} while (0)
 #else
 #include <linux/sysfs.h>
@@ -98,6 +98,12 @@
  return 0;
 }
 
+#define EM8300_I2C_MAKE_LINK(link_name) \
+ do { \
+ if (sysfs_create_link(&em->dev->dev.kobj, &client->dev.kobj, link_name)) \
+ printk(KERN_WARNING "em8300_i2c: unable to create the %s link\n", link_name); \
+ } while (0)
+
 static int em8300_i2c_reg(struct i2c_client *client)
 {
  struct em8300_s *em = i2c_get_adapdata(client->adapter);
@@ -114,7 +120,7 @@
  em->encoder_type = ENCODER_ADV7170;
  }
  em->encoder = client;
- sysfs_create_link(&em->dev->dev.kobj, &client->dev.kobj, "encoder");
+ EM8300_I2C_MAKE_LINK("encoder");
  client->driver->command(client, ENCODER_CMD_ENABLEOUTPUT, (void *)0);
  do {
  struct getconfig_s data;
@@ -169,10 +175,10 @@
  }
  em->encoder_type = ENCODER_BT865;
  em->encoder = client;
- sysfs_create_link(&em->dev->dev.kobj, &client->dev.kobj, "encoder");
+ EM8300_I2C_MAKE_LINK("encoder");
  break;
  case I2C_DRIVERID_EEPROM:
- sysfs_create_link(&em->dev->dev.kobj, &client->dev.kobj, "eeprom");
+ EM8300_I2C_MAKE_LINK("eeprom");
  break;
  default:
  printk(KERN_ERR "em8300_i2c: unknown client id\n");

---

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Nicolas Boullis]

Hi,

On Sun, Jan 06, 2008 at 12:09:23AM +0100, Nicolas Boullis wrote:
>
> That patch is not compatible with the latest changes; so I considered
> writing a small macro to avoid code duplication.
>
> Any opinion about this new patch?

No objection was raised; applied.


Nicolas

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel