[Nut-upsdev] Too much logging from libusb.c (patch supplied)

Sean Conner sean at conman.org
Tue Apr 20 19:43:08 UTC 2010


It was thus said that the Great Arjen de Korte once stated:
> Citeren Sean Conner <sean at conman.org>:
> 
> >  Well, the comment above that function reads:
> >
> >/*
> > * Error handler for usb_get/set_* functions. Return value > 0 success,
> > * 0 unknown or temporary failure (ignored), < 0 permanent failure  
> >(reconnect)
> > */
> 
> And this is exactly what happens.
> 
> >You might want to have a custom message for the 0 case, as usb_strerror()
> >(which I suppose is a wrapper for strerror(), but I haven't checked) just
> >returns "No error" for a value of 0, which to me, is misleading (and the
> >comment says it's ignored anyway, so either the code is wrong, or the
> >comment).  All I saw logged were countless "No error" messages.
> 
> You misunderstood the 'ignored' part here. 

  My point is---I see "No error" being reported, and I think "Gee, they must
be using strerror() and forgot to exclude 0 and look!  They did!"  I'll
admit to missing the comment because (to me) it just kind of gets lost in
the code (and the day I was working with NUT was rather stressed to begin
with, what with having power issues that necessitated the replacement of a
UPS, but I digress).

  Okay, how about *this* patch?

diff --git a/drivers/libusb.c b/drivers/libusb.c
index 50bfc7f..7e7d18d 100644
--- a/drivers/libusb.c
+++ b/drivers/libusb.c
@@ -380,6 +380,10 @@ static int libusb_strerror(const int ret, const char *desc)
                upsdebugx(2, "%s: %s", desc, usb_strerror());
                return 0;
 
+        case 0:                /* do not print out "No error" */
+               upslogx(LOG_DEBUG, "%s: Expected result not received",desc);
+               return 0;
+
        default:        /* Undetermined, log only */
                upslogx(LOG_DEBUG, "%s: %s", desc, usb_strerror());
                return 0;

  The error message logged is a bit more descriptive than just "No error".

> >The problem is I'm monitoring two UPSes via USB and the logging
> >information is insufficient to determine which UPS is giving the bogus
> >information.
> 
> The driver should also log the PID in the log message (at least,  
> that's what it is configured to do so). If you're running multiple  
> drivers, you should still be able to tell them apart by the PID.

  It does; I was just filtering that out in syslog (since I've never found
it useful to have the PID logged anyway, since I'm only running one copy of
the program, or it's easy to know what program failed, etc).  I'm now
logging the PID for usbhid-ups and the problematic driver is the CyberPower
UPS driver (I'm using a Cyber Power 1000AVRLCD).

  -spc (Had I see "Expected result not recieved" I would have realized the
	problem was with the UPS, not the code ... )




More information about the Nut-upsdev mailing list