From 47c68804cbabb9fae8bfaf8b1c2158b13f5b6fe2 Mon Sep 17 00:00:00 2001 From: Chris Frey Date: Thu, 7 Oct 2010 22:35:28 -0400 Subject: [PATCH] Revert "Lib: reading data too small to route to a socket now raises an error instead of just being ignored." This reverts commit c7685942140b123bf110e755ce11a97c3c6372f7. It was discovered that it *is* possible for a BlackBerry to respond with a 0 length USB packet in a bulk read, and it not be an error. So this commit makes an invalid assumption and must be reverted. From Toby Gray's email: On Tue, Oct 05, 2010 at 11:29:47AM +0100, Toby Gray wrote: > When performing testing with some additional devices, I've discovered > that it is possible to receive zero sized reads from some BlackBerry > devices. When using a Curve 8520 it seems to cause zero sized reads > occasionally when it blanks or unblanks the screen. > > I think the only solution is to go back to the old behaviour of ignoring > unroutable reads and just depend on the timeout on sending/receiving to > pick up when the port has been reset. Conflicts: src/error.cc src/error.h src/router.cc --- src/error.cc | 20 -------------- src/error.h | 14 ---------- src/m_raw_channel.cc | 7 ++--- src/router.cc | 77 +++++++++++++++++++--------------------------------- src/router.h | 5 ---- 5 files changed, 30 insertions(+), 93 deletions(-) diff --git a/src/error.cc b/src/error.cc index 0e0d4ed0..22e1484e 100644 --- a/src/error.cc +++ b/src/error.cc @@ -89,25 +89,5 @@ std::string ErrnoError::GetMsg(const std::string &msg, int err) return oss.str(); } - -////////////////////////////////////////////////////////////////////////////// -// UnroutableReadError exception - -UnroutableReadError::UnroutableReadError(unsigned int read_size, - unsigned int min_size) - : Barry::Error(GetMsg(read_size, min_size)) -{ -} - -std::string UnroutableReadError::GetMsg(unsigned int read_size, - unsigned int min_size) -{ - std::ostringstream oss; - oss << "Unroutable read due to size. Data read size: " << read_size - << ". Minimum size: " << min_size; - return oss.str(); -} - - } // namespace Barry diff --git a/src/error.h b/src/error.h index 455d2ec3..23e1fdfe 100644 --- a/src/error.h +++ b/src/error.h @@ -199,20 +199,6 @@ public: }; // -// UnroutableReadError -// -/// Thrown by SocketRoutingQueue when a read is too small to determine -/// the socket, so is unroutable. -/// -class BXEXPORT UnroutableReadError : public Barry::Error -{ - BXLOCAL static std::string GetMsg(unsigned int read_size, - unsigned int min_size); -public: - UnroutableReadError(unsigned int read_size, unsigned int min_size); -}; - -// // BackupError // /// Thrown by the Backup parser class when there is a problem with the diff --git a/src/m_raw_channel.cc b/src/m_raw_channel.cc index 05436e11..67179a7a 100644 --- a/src/m_raw_channel.cc +++ b/src/m_raw_channel.cc @@ -242,14 +242,11 @@ void RawChannel::HandleReceivedData(Data &data) void RawChannel::HandleError(Barry::Error &error) { - std::ostringstream errorOss; - errorOss << "RawChannel: Socket error received, what: " << error.what(); - if( m_callback ) { - m_callback->ChannelError(errorOss.str().c_str()); + m_callback->ChannelError("RawChannel: Socket error received"); } else { - SetPendingError(errorOss.str().c_str()); + SetPendingError("RawChannel: Socket error received"); } m_semaphore->Signal(); } diff --git a/src/router.cc b/src/router.cc index c76e8fab..006492a2 100644 --- a/src/router.cc +++ b/src/router.cc @@ -36,7 +36,7 @@ namespace Barry { void SocketRoutingQueue::SocketDataHandler::Error(Barry::Error &error) { // Just log the error - eout("SocketDataHandler: Error: " << error.what()); + dout("SocketDataHandler: Error: " << error.what()); (void) error; } @@ -195,13 +195,6 @@ bool SocketRoutingQueue::DefaultRead(Data &receive, int timeout) /// DataHandle SocketRoutingQueue::DefaultRead(int timeout) { - if( m_seen_usb_error && timeout == -1 ) - // If an error has been seen and not cleared then no - // more data will be read into the queue by - // DoRead(). Forcing the timeout to zero allows any - // data already in the queue to be read, but prevents - // waiting for data which will never arrive. - timeout = 0; // m_default handles its own locking Data *buf = m_default.wait_pop(timeout); return DataHandle(*this, buf); @@ -417,7 +410,7 @@ void SocketRoutingQueue::DoRead(int timeout) // make sure the size is right if( data.GetSize() < SB_PACKET_SOCKET_SIZE ) - throw UnroutableReadError(data.GetSize(), sizeof(pack->socket)); + return; // bad size, just skip // extract the socket from the packet uint16_t socket = btohs(pack->socket); @@ -462,51 +455,37 @@ void SocketRoutingQueue::DoRead(int timeout) // are able to recover from this error m_seen_usb_error = true; - NotifyHandlersOfError(ue); - } - catch( UnroutableReadError &e ) { - // Although this isn't a USB error the usb error flag - // is set. This is because devices seem to never send - // data which is this small and therefore the only - // time this error is seen (so far) is when the USB - // port has been reset - m_seen_usb_error = true; + // this is unexpected, but we're in a thread here... + // Need to iterate through all the registered handlers + // calling their error callback. + // Can't be locked when calling the callback, so need + // to make a list of them first. + scoped_lock lock(m_mutex); + std::vector handlers; + SocketQueueMap::iterator qi = m_socketQueues.begin(); + while( qi != m_socketQueues.end() ) { + SocketDataHandlerPtr &sdh = qi->second->m_handler; + // is there a handler? + if( sdh ) { + handlers.push_back(sdh); + } + ++qi; + } - NotifyHandlersOfError(e); - } -} + SocketDataHandlerPtr usb_error_handler = m_usb_error_dev_callback; -void SocketRoutingQueue::NotifyHandlersOfError(Barry::Error &error) -{ - // Need to iterate through all the registered handlers - // calling their error callback. - // Can't be locked when calling the callback, so need - // to make a list of them first. - scoped_lock lock(m_mutex); - std::vector handlers; - SocketQueueMap::iterator qi = m_socketQueues.begin(); - while( qi != m_socketQueues.end() ) { - SocketDataHandlerPtr &sdh = qi->second->m_handler; - // is there a handler? - if( sdh ) { - handlers.push_back(sdh); + lock.unlock(); + std::vector::iterator hi = handlers.begin(); + while( hi != handlers.end() ) { + (*hi)->Error(ue); + ++hi; } - ++qi; - } - - SocketDataHandlerPtr usb_error_handler = m_usb_error_dev_callback; - lock.unlock(); - std::vector::iterator hi = handlers.begin(); - while( hi != handlers.end() ) { - (*hi)->Error(error); - ++hi; + // and finally, call the specific error callback if available + if( usb_error_handler.get() ) { + usb_error_handler->Error(ue); + } } - - // and finally, call the specific error callback if available - if( usb_error_handler.get() ) { - usb_error_handler->Error(error); - } } void SocketRoutingQueue::SpinoffSimpleReadThread() diff --git a/src/router.h b/src/router.h index 77351274..04f32cf0 100644 --- a/src/router.h +++ b/src/router.h @@ -132,11 +132,6 @@ protected: // created in the SpinoffSimpleReadThread() member below. static void *SimpleReadThread(void *userptr); - // Notifies all registered handlers of an error. - // Intended to be called by error handling code within threads which - // aren't the main thread. - void NotifyHandlersOfError(Barry::Error &error); - public: SocketRoutingQueue(int prealloc_buffer_count = 4); ~SocketRoutingQueue(); -- 2.11.4.GIT