From 4ee4bbdb58d7dabae1be1fa6e62622caa5a4fcbc Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 13 Mar 2020 10:06:06 -0700 Subject: [PATCH] iostream: fix incorrect rfcomm error case when reading We had two independent bugs here, both of which needed to fire for this to cause a problem. This fixes both of them. The first bug was that our rfcomm code would return DC_STATUS_SUCCESS with a zero-sized read when a timeout happened, or when the rfcomm socket had disconnected. That makes absolutely no sense. We should return DC_STATUS_TIMEOUT on timeout, and DC_STATUS_IO if the socket has disconnected without any data. The fix to this is to make the whole rfcomm iostream read logic much simpler: there's no need to loop at all for partial results, because the libdivecomputer iostream side will do the loop for us (and handle partial results much better: it knows if the target backend can handle those partial results or not). The second bug was in our libdivecomputer iostream read() function, which reacted very badly to this bad return value. This updates our libdivecomputer branch to one that is more careful about things. Reported-by: linuxcrash Debugged-by: Jef Driesen Signed-off-by: Linus Torvalds --- core/qtserialbluetooth.cpp | 51 ++++++++++++++++++-------------------- libdivecomputer | 2 +- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/core/qtserialbluetooth.cpp b/core/qtserialbluetooth.cpp index 6b47524de..f6bbcafdf 100644 --- a/core/qtserialbluetooth.cpp +++ b/core/qtserialbluetooth.cpp @@ -133,42 +133,39 @@ static dc_status_t qt_serial_read(void *io, void* data, size_t size, size_t *act { qt_serial_t *device = (qt_serial_t*) io; - if (device == NULL || device->socket == NULL) + if (device == NULL || device->socket == NULL || !actual) return DC_STATUS_INVALIDARGS; - size_t nbytes = 0; - int rc; + *actual = 0; + for (;;) { + int rc; - while(nbytes < size && device->socket->state() == QBluetoothSocket::ConnectedState) - { - rc = device->socket->read((char *) data + nbytes, size - nbytes); + if (device->socket->state() != QBluetoothSocket::ConnectedState) + return DC_STATUS_IO; + rc = device->socket->read((char *) data, size); if (rc < 0) { if (errno == EINTR || errno == EAGAIN) - continue; // Retry. - - return DC_STATUS_IO; // Something really bad happened :-( - } else if (rc == 0) { - // Wait until the device is available for read operations - QEventLoop loop; - QTimer timer; - timer.setSingleShot(true); - loop.connect(&timer, SIGNAL(timeout()), SLOT(quit())); - loop.connect(device->socket, SIGNAL(readyRead()), SLOT(quit())); - timer.start(device->timeout); - loop.exec(); - - if (!timer.isActive()) - break; + continue; + return DC_STATUS_IO; } - nbytes += rc; + *actual = rc; + if (rc > 0 || !size) + return DC_STATUS_SUCCESS; + + // Timeout handling + QEventLoop loop; + QTimer timer; + timer.setSingleShot(true); + loop.connect(&timer, SIGNAL(timeout()), SLOT(quit())); + loop.connect(device->socket, SIGNAL(readyRead()), SLOT(quit())); + timer.start(device->timeout); + loop.exec(); + + if (!timer.isActive()) + return DC_STATUS_TIMEOUT; } - - if (actual) - *actual = nbytes; - - return DC_STATUS_SUCCESS; } static dc_status_t qt_serial_write(void *io, const void* data, size_t size, size_t *actual) diff --git a/libdivecomputer b/libdivecomputer index 0714e327b..b77e7b6b2 160000 --- a/libdivecomputer +++ b/libdivecomputer @@ -1 +1 @@ -Subproject commit 0714e327b70bb08f5289c9781c09dc10884881c9 +Subproject commit b77e7b6b217860e166081f8a93fb8bac81797897