From a1c408e8f44dade88dd3f49d0a5a439b5c641d90 Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Thu, 7 May 2020 13:45:03 -0700
Subject: [PATCH] Make custom iostream read/write wrapper more rebust

The dc_iostream_{read,write}() implementation had multiple issues:

 - it would return DC_STATUS_SUCCESS even if no iostream implementation
   existed.

   Yes, it would also return a zero "actual" bytes, but most backends
   don't even pass an "actual" pointer, so returning success was still
   completely insane.

   This one probably didn't matter, because all iostreams should have
   read and write members, but the return value was completely wrong if
   that ever were to happen.

 - If the user passed in a NULL 'actual' pointer, the wrapper would
   ignore that, and pass in its own pointer instead, in order to know
   how many bytes to print for the debug message.

   But that means that the low-level read/write functions cannot know
   whether the user actually is able to handle a partial read or not.

   This one _definitely_ matters, because some protocols need to have a
   buffer for the whole incoming packet, but packerts may not always be
   full-size. The low-level protocol needs to know whether to wait for
   further packets (in order to fill the buffer) or to just return the
   partial data.

This fixes these issues.  If the user passes in a NULL actual pointer
(indicating that it needs all-or-nothing and is not ready to handle a
partial success), just loop over the IO until the buffer is fully
exhausted.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 src/iostream.c | 95 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 69 insertions(+), 26 deletions(-)

diff --git a/src/iostream.c b/src/iostream.c
index 51713d1..376c9e4 100644
--- a/src/iostream.c
+++ b/src/iostream.c
@@ -188,43 +188,86 @@ dc_iostream_poll (dc_iostream_t *iostream, int timeout)
 dc_status_t
 dc_iostream_read (dc_iostream_t *iostream, void *data, size_t size, size_t *actual)
 {
-	dc_status_t status = DC_STATUS_SUCCESS;
-	size_t nbytes = 0;
+	if (actual)
+		*actual = 0;
 
-	if (iostream == NULL || iostream->vtable->read == NULL) {
-		goto out;
+	if (iostream == NULL || iostream->vtable->read == NULL)
+		return DC_STATUS_IO;
+
+	while (size) {
+		dc_status_t status;
+		size_t nbytes = 0;
+
+		status = iostream->vtable->read (iostream, data, size, &nbytes);
+		HEXDUMP (iostream->context, DC_LOGLEVEL_INFO, "Read", (unsigned char *) data, nbytes);
+
+		/*
+		 * If the reader is able to handle partial results,
+		 * return them as such. NOTE! No need to add up a
+		 * total, we will go through this loop only once
+		 * in this case.
+		 */
+		if (actual) {
+			*actual = nbytes;
+			return status;
+		}
+
+		if (status != DC_STATUS_SUCCESS)
+			return status;
+
+		/*
+		 * Defensive check: if the read() function returned
+		 * zero bytes, don't loop forever - give up with a
+		 * timeout. Jef pointed out that the subsurface
+		 * qt_serial_read() function can cause this badness..
+		 */
+		if (!nbytes)
+			return DC_STATUS_TIMEOUT;
+
+		/*
+		 * Continue reading to fill up the whole buffer,
+		 * since the reader is not able to handle a
+		 * partial result.
+		 */
+		data = (void *)(nbytes + (char *)data);
+		size -= nbytes;
 	}
 
-	status = iostream->vtable->read (iostream, data, size, &nbytes);
-
-	HEXDUMP (iostream->context, DC_LOGLEVEL_INFO, "Read", (unsigned char *) data, nbytes);
-
-out:
-	if (actual)
-		*actual = nbytes;
-
-	return status;
+	return DC_STATUS_SUCCESS;
 }
 
 dc_status_t
 dc_iostream_write (dc_iostream_t *iostream, const void *data, size_t size, size_t *actual)
 {
-	dc_status_t status = DC_STATUS_SUCCESS;
-	size_t nbytes = 0;
+	if (actual)
+		*actual = 0;
 
-	if (iostream == NULL || iostream->vtable->write == NULL) {
-		goto out;
+	if (iostream == NULL || iostream->vtable->write == NULL)
+		return DC_STATUS_IO;
+
+	while (size) {
+		dc_status_t status;
+		size_t nbytes = 0;
+
+		status = iostream->vtable->write (iostream, data, size, &nbytes);
+		HEXDUMP (iostream->context, DC_LOGLEVEL_INFO, "Write", (const unsigned char *) data, nbytes);
+
+		if (actual) {
+			*actual = nbytes;
+			return status;
+		}
+
+		if (status != DC_STATUS_SUCCESS)
+			return status;
+
+		if (!nbytes)
+			return DC_STATUS_IO;
+
+		data = (void *)(nbytes + (char *)data);
+		size -= nbytes;
 	}
 
-	status = iostream->vtable->write (iostream, data, size, &nbytes);
-
-	HEXDUMP (iostream->context, DC_LOGLEVEL_INFO, "Write", (const unsigned char *) data, nbytes);
-
-out:
-	if (actual)
-		*actual = nbytes;
-
-	return status;
+	return DC_STATUS_SUCCESS;
 }
 
 dc_status_t