[lm-sensors] [Patch] libsensors: Introduce error string - Unknown

[lm-sensors] [Patch] libsensors: Introduce error string - Unknown

[Andre Prendel]

Hello,

sensors_error() returns NULL if errnum is not valid. Returning
"Unknown error" in this case, makes output clearer than:

foo: (null)

and prevents error output/logging from segmentation faults.

What do you think?

Andre

Signed-off-by: Andre Prendel <andre_prendel@gmx.de>
---

--- lm-sensors-dev/lib/error.c	2009-01-26 17:43:43.000000000 +0100
+++ my-lm-sensors/lib/error.c	2009-01-28 22:49:53.000000000 +0100
@@ -33,7 +33,7 @@ void (*sensors_fatal_error) (const char 
 						sensors_default_fatal_error;
 
 static const char *errorlist[] = {
-	/* Invalid error code    */ NULL,
+	/* Invalid error code    */ "Unknown error",
 	/* SENSORS_ERR_WILDCARDS */ "Wildcard found in chip name",
 	/* SENSORS_ERR_NO_ENTRY  */ "No such subfeature known",
 	/* SENSORS_ERR_ACCESS_R  */ "Can't read",

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [Patch] libsensors: Introduce error string -

[Jean Delvare]

Hi Andre,

On Thu, 29 Jan 2009 10:09:45 +0100, Andre Prendel wrote:
> sensors_error() returns NULL if errnum is not valid. Returning
> "Unknown error" in this case, makes output clearer than:
> 
> foo: (null)
> 
> and prevents error output/logging from segmentation faults.
> 
> What do you think?
> 
> Andre
> 
> Signed-off-by: Andre Prendel <andre_prendel@gmx.de>
> ---
> 
> --- lm-sensors-dev/lib/error.c	2009-01-26 17:43:43.000000000 +0100
> +++ my-lm-sensors/lib/error.c	2009-01-28 22:49:53.000000000 +0100
> @@ -33,7 +33,7 @@ void (*sensors_fatal_error) (const char 
>  						sensors_default_fatal_error;
>  
>  static const char *errorlist[] = {
> -	/* Invalid error code    */ NULL,
> +	/* Invalid error code    */ "Unknown error",
>  	/* SENSORS_ERR_WILDCARDS */ "Wildcard found in chip name",
>  	/* SENSORS_ERR_NO_ENTRY  */ "No such subfeature known",
>  	/* SENSORS_ERR_ACCESS_R  */ "Can't read",

Out of curiosity: did you actually hit this problem in real life?

I was about to say that the caller was supposed to check for NULL, but
apparently neither sensors nor sensord nor even libsensors does, so...
I guess you're right, having an error string instead of NULL would be
better. Actually that's what libsensors 2.x did, I can't remember why I
changed that in 3.x:
http://www.lm-sensors.org/changeset/4854

Probably I thought it was not supposed to happen, but I guess it may
actually happen if mixing a recent application with an older version of
libsensors.

So I am pretty much inclined to apply your patch.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [Patch] libsensors: Introduce error string

[Andre Prendel]

On Thu, Jan 29, 2009 at 10:57:03AM +0100, Jean Delvare wrote:
> Hi Andre,
> 
> On Thu, 29 Jan 2009 10:09:45 +0100, Andre Prendel wrote:
> > sensors_error() returns NULL if errnum is not valid. Returning
> > "Unknown error" in this case, makes output clearer than:
> > 
> > foo: (null)
> > 
> > and prevents error output/logging from segmentation faults.
> > 
> > What do you think?
> > 
> > Andre
> > 
> > Signed-off-by: Andre Prendel <andre_prendel@gmx.de>
> > ---
> > 
> > --- lm-sensors-dev/lib/error.c	2009-01-26 17:43:43.000000000 +0100
> > +++ my-lm-sensors/lib/error.c	2009-01-28 22:49:53.000000000 +0100
> > @@ -33,7 +33,7 @@ void (*sensors_fatal_error) (const char 
> >  						sensors_default_fatal_error;
> >  
> >  static const char *errorlist[] = {
> > -	/* Invalid error code    */ NULL,
> > +	/* Invalid error code    */ "Unknown error",
> >  	/* SENSORS_ERR_WILDCARDS */ "Wildcard found in chip name",
> >  	/* SENSORS_ERR_NO_ENTRY  */ "No such subfeature known",
> >  	/* SENSORS_ERR_ACCESS_R  */ "Can't read",
> 
> Out of curiosity: did you actually hit this problem in real life?

No, I didn't. It's just theoretical.
 
> I was about to say that the caller was supposed to check for NULL, but
> apparently neither sensors nor sensord nor even libsensors does, so...

IMO error reporting should be very stable. If the caller checks
return value carefully it's fine too. Fprintf() isn't a problem. But I
don't know what about vsnprintf(), syslog().

> I guess you're right, having an error string instead of NULL would be
> better. Actually that's what libsensors 2.x did, I can't remember why I
> changed that in 3.x:
> http://www.lm-sensors.org/changeset/4854
> 
> Probably I thought it was not supposed to happen, but I guess it may
> actually happen if mixing a recent application with an older version of
> libsensors.
> 
> So I am pretty much inclined to apply your patch.

Vous avez le choix. :)

Andre

> -- 
> Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [Patch] libsensors: Introduce error string -

[Jean Delvare]

On Thu, 29 Jan 2009 12:12:28 +0100, Andre Prendel wrote:
> On Thu, Jan 29, 2009 at 10:57:03AM +0100, Jean Delvare wrote:
> > Hi Andre,
> > 
> > On Thu, 29 Jan 2009 10:09:45 +0100, Andre Prendel wrote:
> > > sensors_error() returns NULL if errnum is not valid. Returning
> > > "Unknown error" in this case, makes output clearer than:
> > > 
> > > foo: (null)
> > > 
> > > and prevents error output/logging from segmentation faults.
> > > 
> > > What do you think?
> > > 
> > > Andre
> > > 
> > > Signed-off-by: Andre Prendel <andre_prendel@gmx.de>
> > > ---
> > > 
> > > --- lm-sensors-dev/lib/error.c	2009-01-26 17:43:43.000000000 +0100
> > > +++ my-lm-sensors/lib/error.c	2009-01-28 22:49:53.000000000 +0100
> > > @@ -33,7 +33,7 @@ void (*sensors_fatal_error) (const char 
> > >  						sensors_default_fatal_error;
> > >  
> > >  static const char *errorlist[] = {
> > > -	/* Invalid error code    */ NULL,
> > > +	/* Invalid error code    */ "Unknown error",
> > >  	/* SENSORS_ERR_WILDCARDS */ "Wildcard found in chip name",
> > >  	/* SENSORS_ERR_NO_ENTRY  */ "No such subfeature known",
> > >  	/* SENSORS_ERR_ACCESS_R  */ "Can't read",
> > 
> > Out of curiosity: did you actually hit this problem in real life?
> 
> No, I didn't. It's just theoretical.
>  
> > I was about to say that the caller was supposed to check for NULL, but
> > apparently neither sensors nor sensord nor even libsensors does, so...
> 
> IMO error reporting should be very stable. If the caller checks
> return value carefully it's fine too. Fprintf() isn't a problem. But I
> don't know what about vsnprintf(), syslog().
> 
> > I guess you're right, having an error string instead of NULL would be
> > better. Actually that's what libsensors 2.x did, I can't remember why I
> > changed that in 3.x:
> > http://www.lm-sensors.org/changeset/4854
> > 
> > Probably I thought it was not supposed to happen, but I guess it may
> > actually happen if mixing a recent application with an older version of
> > libsensors.
> > 
> > So I am pretty much inclined to apply your patch.
> 
> Vous avez le choix. :)

Sicherlich :)

Patch applied, thanks. If you intend to contribute to the lm-sensors
project again on a regular basis, please let me know, we can create an
account for you.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [Patch] libsensors: Introduce error string

[Andre Prendel]

On Thu, Jan 29, 2009 at 01:28:19PM +0100, Jean Delvare wrote:
> On Thu, 29 Jan 2009 12:12:28 +0100, Andre Prendel wrote:
> > On Thu, Jan 29, 2009 at 10:57:03AM +0100, Jean Delvare wrote:
> > > Hi Andre,
> > > 
> > > On Thu, 29 Jan 2009 10:09:45 +0100, Andre Prendel wrote:
> > > > sensors_error() returns NULL if errnum is not valid. Returning
> > > > "Unknown error" in this case, makes output clearer than:
> > > > 
> > > > foo: (null)
> > > > 
> > > > and prevents error output/logging from segmentation faults.
> > > > 
> > > > What do you think?
> > > > 
> > > > Andre
> > > > 
> > > > Signed-off-by: Andre Prendel <andre_prendel@gmx.de>
> > > > ---
> > > > 
> > > > --- lm-sensors-dev/lib/error.c	2009-01-26 17:43:43.000000000 +0100
> > > > +++ my-lm-sensors/lib/error.c	2009-01-28 22:49:53.000000000 +0100
> > > > @@ -33,7 +33,7 @@ void (*sensors_fatal_error) (const char 
> > > >  						sensors_default_fatal_error;
> > > >  
> > > >  static const char *errorlist[] = {
> > > > -	/* Invalid error code    */ NULL,
> > > > +	/* Invalid error code    */ "Unknown error",
> > > >  	/* SENSORS_ERR_WILDCARDS */ "Wildcard found in chip name",
> > > >  	/* SENSORS_ERR_NO_ENTRY  */ "No such subfeature known",
> > > >  	/* SENSORS_ERR_ACCESS_R  */ "Can't read",
> > > 
> > > Out of curiosity: did you actually hit this problem in real life?
> > 
> > No, I didn't. It's just theoretical.
> >  
> > > I was about to say that the caller was supposed to check for NULL, but
> > > apparently neither sensors nor sensord nor even libsensors does, so...
> > 
> > IMO error reporting should be very stable. If the caller checks
> > return value carefully it's fine too. Fprintf() isn't a problem. But I
> > don't know what about vsnprintf(), syslog().
> > 
> > > I guess you're right, having an error string instead of NULL would be
> > > better. Actually that's what libsensors 2.x did, I can't remember why I
> > > changed that in 3.x:
> > > http://www.lm-sensors.org/changeset/4854
> > > 
> > > Probably I thought it was not supposed to happen, but I guess it may
> > > actually happen if mixing a recent application with an older version of
> > > libsensors.
> > > 
> > > So I am pretty much inclined to apply your patch.
> > 
> > Vous avez le choix. :)
> 
> Sicherlich :)
> 
> Patch applied, thanks. If you intend to contribute to the lm-sensors
> project again on a regular basis, please let me know, we can create an
> account for you.

I'm very interested in contributing more stuff to the project. For the
moment I'm happy sending patches to you. If I should be more involved in
the future we can talk about an account again.

Thanks
Andre
 
> -- 
> Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors