Tuesday, April 20, 2010
When “No error” is actually an error
My patch to NUT was rejected:
No.
The above is an error condition (despite the 'No error' message), most likely due to buggy UPS firmware. Normally, we should not expect that when asking for a report, the UPS returns nothing. After all it is 'advertising' the report in the report descriptor, so silently ignoring this would be a grievous mistake. At the very least, if someone is debugging the we should provide some indication why this fails.
Hmm … okay. I thought they just missed typed a conditional, since 0 is used to indicate success throughout a mess of Standard C library (and Unix library) calls (silly me!).
The cause of the “No error” message is this bit of code:
/*
* Error handler for usb_get/set_* functions. Return value > 0 success,
* 0 unknown or temporary failure (ignored), < 0 permanent failure (reconnect)
*/
static int libusb_strerror(const int ret, const char *desc)
{
if (ret > 0) {
return ret;
}
switch(ret)
{
case -EBUSY: /* Device or resource busy */
case -EPERM: /* Operation not permitted */
case -ENODEV: /* No such device */
case -EACCES: /* Permission denied */
case -EIO: /* I/O error */
case -ENXIO: /* No such device or address */
case -ENOENT: /* No such file or directory */
case -EPIPE: /* Broken pipe */
case -ENOSYS: /* Function not implemented */
upslogx(LOG_DEBUG, "%s: %s", desc, usb_strerror());
return ret;
case -ETIMEDOUT: /* Connection timed out */
upsdebugx(2, "%s: Connection timed out", desc);
return 0;
case -EOVERFLOW: /* Value too large for defined data type */
case -EPROTO: /* Protocol error */
upsdebugx(2, "%s: %s", desc, usb_strerror());
return 0;
default: /* Undetermined, log only */
upslogx(LOG_DEBUG, "%s: %s", desc, usb_strerror());
return 0;
}
}
While I have yet to find the code for usb_strerror() (and
I've searched every file; I have no clue as to where the
definition of usb_strerror() is located), it acts as if it's
just a wrapper around strerror() (a Standard C library call),
and when given a value of 0, it returns “No error” (since 0 isn't
considered an error value). I submitted back a patch to print “Expected
result not received”, since that seems to be what a 0 result means.
Also notice that the comment describing the results is somewhat lost at the top there—in the actual code it's even more invisible since there isn't much to visually set it off from the rest of the code.
Hopefully, the new patch I submitted will be accepted.
