From 9458a0a7c1452af58a782f8d4d92ce0af1033394 Mon Sep 17 00:00:00 2001 From: Chris Frey Date: Fri, 17 Jun 2011 18:30:01 -0400 Subject: [PATCH] lib: changed Usb::Error exception to deal in libusb_errcode instead of system No error information should ever be lost, and in the libusb translation code, there was a slight possibility that it could be lost. This commit makes sure that the Usb::Error exception always stores the low level errcode, and adds a system_errcode() function to translate it for error handling purposes, such as for common errors like -EBUSY. --- gui/src/main.cc | 2 +- src/probe.cc | 2 +- src/usbwrap.cc | 23 +++++++++++++------- src/usbwrap.h | 11 +++++++--- src/usbwrap_libusb.cc | 8 ++++++- src/usbwrap_libusb_1_0.cc | 55 +++++++++++++++++++---------------------------- 6 files changed, 54 insertions(+), 47 deletions(-) diff --git a/gui/src/main.cc b/gui/src/main.cc index e02b4c3e..199ddbd7 100644 --- a/gui/src/main.cc +++ b/gui/src/main.cc @@ -50,7 +50,7 @@ void main_exception_handler() // special check for EBUSY to make the error message // more user friendly Gtk::MessageDialog msg(""); - if( e.errcode() == -EBUSY ) { + if( e.system_errcode() == -EBUSY ) { msg.set_message("Device busy. This is likely due to the usb_storage kernel module being loaded. Try 'rmmod usb_storage'."); } else { diff --git a/src/probe.cc b/src/probe.cc index a4418421..8e98abcd 100644 --- a/src/probe.cc +++ b/src/probe.cc @@ -173,7 +173,7 @@ void Probe::ProbeMatching(int vendor, int product, } catch( Usb::Error &e ) { dout("Usb::Error exception caught: " << e.what()); - if( e.errcode() == -EBUSY ) { + if( e.system_errcode() == -EBUSY ) { m_fail_count++; m_fail_msgs.push_back(e.what()); } diff --git a/src/usbwrap.cc b/src/usbwrap.cc index c389a903..0510bf2f 100644 --- a/src/usbwrap.cc +++ b/src/usbwrap.cc @@ -53,31 +53,38 @@ namespace Usb { /////////////////////////////////////////////////////////////////////////////// // Usb::Error exception class -static std::string GetErrorString(int errcode, const std::string &str) +static std::string GetErrorString(int libusb_errcode, const std::string &str) { std::ostringstream oss; oss << "("; - if( errcode ) { - oss << std::setbase(10) << errcode << ", "; + if( libusb_errcode ) { + oss << std::dec << libusb_errcode << " (" + << LibraryInterface::TranslateErrcode(libusb_errcode) + << "), "; } - oss << LibraryInterface::GetLastErrorString(errcode) << "): "; + oss << LibraryInterface::GetLastErrorString(libusb_errcode) << "): "; oss << str; return oss.str(); } Error::Error(const std::string &str) : Barry::Error(GetErrorString(0, str)) - , m_errcode(0) + , m_libusb_errcode(0) { } -Error::Error(int errcode, const std::string &str) - : Barry::Error(GetErrorString(errcode, str)) - , m_errcode(errcode) +Error::Error(int libusb_errcode, const std::string &str) + : Barry::Error(GetErrorString(libusb_errcode, str)) + , m_libusb_errcode(libusb_errcode) { } +int Error::system_errcode() const +{ + return LibraryInterface::TranslateErrcode(m_libusb_errcode); +} + /////////////////////////////////////////////////////////////////////////////// // EndpointPair diff --git a/src/usbwrap.h b/src/usbwrap.h index 0f4ac178..ee60b415 100644 --- a/src/usbwrap.h +++ b/src/usbwrap.h @@ -50,14 +50,18 @@ namespace Usb { /// Thrown on low level USB errors. class BXEXPORT Error : public Barry::Error { - int m_errcode; + int m_libusb_errcode; public: Error(const std::string &str); Error(int errcode, const std::string &str); // can return 0 in some case, if unknown error code - int errcode() const { return m_errcode; } + int libusb_errcode() const { return m_libusb_errcode; } + + // returns a translated system error code when using libusb 1.0 + // returns 0 if unknown or unable to translate + int system_errcode() const; }; class BXEXPORT Timeout : public Error @@ -84,7 +88,8 @@ struct DeviceHandle; class BXEXPORT LibraryInterface { public: - static std::string GetLastErrorString(int errcode); + static std::string GetLastErrorString(int libusb_errcode); + static int TranslateErrcode(int libusb_errcode); static int Init(); static void Uninit(); static void SetDataDump(bool data_dump_mode); diff --git a/src/usbwrap_libusb.cc b/src/usbwrap_libusb.cc index cb4789e0..138f7f8f 100644 --- a/src/usbwrap_libusb.cc +++ b/src/usbwrap_libusb.cc @@ -48,12 +48,18 @@ template static void deleteMapPtr(std::pair ptr) { /////////////////////////////////////////////////////////////////////////////// // Static functions -std::string LibraryInterface::GetLastErrorString(int /*errcode*/) +std::string LibraryInterface::GetLastErrorString(int /*libusb_errcode*/) { // Errcode is unused by libusb, so just call the last error return std::string(usb_strerror()); } +int LibraryInterface::TranslateErrcode(int libusb_errcode) +{ + // libusb errcode == system errcode + return libusb_errcode; +} + int LibraryInterface::Init() { // if the environment variable USB_DEBUG is set, that diff --git a/src/usbwrap_libusb_1_0.cc b/src/usbwrap_libusb_1_0.cc index 98c4512d..0ebb7fbe 100644 --- a/src/usbwrap_libusb_1_0.cc +++ b/src/usbwrap_libusb_1_0.cc @@ -72,18 +72,6 @@ static const struct { static const int errorCodeCnt = sizeof(errorCodes) / sizeof(errorCodes[0]); -// helper function to translate libusb error codes into more useful values -static int translateError(int err) -{ - for( int i = 0; i < errorCodeCnt; ++i ) { - if( errorCodes[i].libusb == err ) - return errorCodes[i].system; - } - // default to passing unknown errors out - return err; -} - - /////////////////////////////////////////////////////////////////////////////// // Global libusb library context @@ -92,19 +80,9 @@ static libusb_context* libusbctx; /////////////////////////////////////////////////////////////////////////////// // Static functions -std::string LibraryInterface::GetLastErrorString(int errcode) +std::string LibraryInterface::GetLastErrorString(int libusb_errcode) { - // This takes the error codes as translated using the - // translateError function above, so need to convert it - // back first - int libusberr = errcode; - for( int i = 0; i < errorCodeCnt; ++i ) { - if( errorCodes[i].system == errcode ) { - libusberr = errorCodes[i].libusb; - break; - } - } - switch(libusberr) + switch( libusb_errcode ) { case LIBUSB_SUCCESS: return "Success"; @@ -139,6 +117,18 @@ std::string LibraryInterface::GetLastErrorString(int errcode) } } +// helper function to translate libusb error codes into more useful values +int LibraryInterface::TranslateErrcode(int libusb_errcode) +{ + for( int i = 0; i < errorCodeCnt; ++i ) { + if( errorCodes[i].libusb == libusb_errcode ) + return errorCodes[i].system; + } + + // default to 0 if unknown + return 0; +} + int LibraryInterface::Init() { // if the environment variable LIBUSB_DEBUG is set, that @@ -337,13 +327,12 @@ bool Device::BulkRead(int ep, Barry::Data &data, int timeout) // otherwise treat it as success. if( ret == LIBUSB_ERROR_TIMEOUT ) { if( transferred == 0 ) - throw Timeout(translateError(ret), - "Timeout in BulkRead"); + throw Timeout(ret, "Timeout in BulkRead"); else dout("Read timed out with some data transferred... possible partial read"); } else if( ret != LIBUSB_ERROR_TIMEOUT ) - throw Error(translateError(ret), "Error in BulkRead"); + throw Error(ret, "Error in BulkRead"); } if( transferred != 0 ) data.ReleaseBuffer(transferred); @@ -372,7 +361,7 @@ bool Device::BulkWrite(int ep, const Barry::Data &data, int timeout) if( ret == LIBUSB_ERROR_TIMEOUT && transferred == 0 ) throw Timeout(ret, "Timeout in BulkWrite"); else if( ret != LIBUSB_ERROR_TIMEOUT ) - throw Error(translateError(ret), "Error in BulkWrite"); + throw Error(ret, "Error in BulkWrite"); } if( ret >= 0 && (unsigned int)transferred != data.GetSize() ) { @@ -409,7 +398,7 @@ bool Device::BulkWrite(int ep, const void *data, size_t size, int timeout) if( ret == LIBUSB_ERROR_TIMEOUT && transferred == 0 ) throw Timeout(ret, "Timeout in BulkWrite (2)"); else if( ret != LIBUSB_ERROR_TIMEOUT ) - throw Error(translateError(ret), "Error in BulkWrite (2)"); + throw Error(ret, "Error in BulkWrite (2)"); } if( ret >= 0 && (unsigned int)transferred != size ) { dout("Failed to write all data on ep: " << ep << @@ -446,7 +435,7 @@ bool Device::InterruptRead(int ep, Barry::Data &data, int timeout) dout("Read timed out with some data transferred... possible partial read"); } else if( ret != LIBUSB_ERROR_TIMEOUT ) - throw Error(translateError(ret), "Error in InterruptRead"); + throw Error(ret, "Error in InterruptRead"); } if( transferred != 0 ) data.ReleaseBuffer(transferred); @@ -474,9 +463,9 @@ bool Device::InterruptWrite(int ep, const Barry::Data &data, int timeout) // only notify of a timeout if no data was transferred, // otherwise treat it as success. if( ret == LIBUSB_ERROR_TIMEOUT && transferred == 0 ) - throw Timeout(translateError(ret), "Timeout in InterruptWrite"); + throw Timeout(ret, "Timeout in InterruptWrite"); else if( ret != LIBUSB_ERROR_TIMEOUT ) - throw Error(translateError(ret), "Error in InterruptWrite"); + throw Error(ret, "Error in InterruptWrite"); } if( ret >= 0 && (unsigned int)transferred != data.GetSize() ) { @@ -593,7 +582,7 @@ Interface::Interface(Device &dev, int iface) dout("libusb_claim_interface(" << dev.GetHandle()->m_handle << ", 0x" << std::hex << iface << ")"); int ret = libusb_claim_interface(dev.GetHandle()->m_handle, iface); if( ret < 0 ) - throw Error(translateError(ret), "claim interface failed"); + throw Error(ret, "claim interface failed"); } Interface::~Interface() -- 2.11.4.GIT