[Asterisk-code-review] iostream.c: Fix ast iostream gets() needlessly returning fai... (asterisk[16])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/10028 )

Change subject: iostream.c: Fix ast_iostream_gets() needlessly returning failure.
......................................................................

iostream.c: Fix ast_iostream_gets() needlessly returning failure.

Providing a buffer larger than the internal buffer of ast_iostream_gets()
fails to get lines longer than the internal buffer.

* Made ast_iostream_gets() fill the supplied buffer with read data until
either a '\n' is found or the supplied buffer is filled just like fgets().

Change-Id: If18b3f6ee500e22f0633a68779ed09f7e0f305ed
---
M main/iostream.c
1 file changed, 36 insertions(+), 23 deletions(-)

Approvals:
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved
  George Joseph: Approved for Submit



diff --git a/main/iostream.c b/main/iostream.c
index 819616a..15131c0 100644
--- a/main/iostream.c
+++ b/main/iostream.c
@@ -285,46 +285,59 @@
 
 ssize_t ast_iostream_gets(struct ast_iostream *stream, char *buffer, size_t size)
 {
-	ssize_t r;
+	size_t remaining = size;
+	ssize_t accum_size = 0;
+	ssize_t len;
 	char *newline;
 
-	do {
+	for (;;) {
 		/* Search for newline */
 		newline = memchr(stream->rbufhead, '\n', stream->rbuflen);
 		if (newline) {
-			r = newline - stream->rbufhead + 1;
-			if (r > size-1) {
-				r = size-1;
+			len = newline - stream->rbufhead + 1;
+			if (len > remaining - 1) {
+				len = remaining - 1;
 			}
 			break;
 		}
 
-		/* Enough data? */
-		if (stream->rbuflen >= size - 1) {
-			r = size - 1;
+		/* Enough buffered line data to fill request buffer? */
+		if (stream->rbuflen >= remaining - 1) {
+			len = remaining - 1;
 			break;
 		}
-
-		/* Try to fill in line buffer */
-		if (stream->rbuflen && stream->rbuf != stream->rbufhead) {
-			memmove(&stream->rbuf, stream->rbufhead, stream->rbuflen);
+		if (stream->rbuflen) {
+			/* Put leftover buffered line data into request buffer */
+			memcpy(buffer + accum_size, stream->rbufhead, stream->rbuflen);
+			remaining -= stream->rbuflen;
+			accum_size += stream->rbuflen;
+			stream->rbuflen = 0;
 		}
 		stream->rbufhead = stream->rbuf;
 
-		r = iostream_read(stream, stream->rbufhead + stream->rbuflen, sizeof(stream->rbuf) - stream->rbuflen);
-		if (r <= 0) {
-			return r;
+		len = iostream_read(stream, stream->rbuf, sizeof(stream->rbuf));
+		if (len == 0) {
+			/* Nothing new was read.  Return whatever we have accumulated. */
+			break;
 		}
-		stream->rbuflen += r;
-	} while (1);
+		if (len < 0) {
+			if (accum_size) {
+				/* We have an accumulated buffer so return that instead. */
+				len = 0;
+				break;
+			}
+			return len;
+		}
+		stream->rbuflen += len;
+	}
 
-	/* Return r bytes with termination byte */
-	memcpy(buffer, stream->rbufhead, r);
-	buffer[r] = 0;
-	stream->rbuflen -= r;
-	stream->rbufhead += r;
+	/* Return read buffer string length */
+	memcpy(buffer + accum_size, stream->rbufhead, len);
+	buffer[accum_size + len] = 0;
+	stream->rbuflen -= len;
+	stream->rbufhead += len;
 
-	return r;
+	return accum_size + len;
 }
 
 ssize_t ast_iostream_discard(struct ast_iostream *stream, size_t size)

-- 
To view, visit https://gerrit.asterisk.org/10028
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-MessageType: merged
Gerrit-Change-Id: If18b3f6ee500e22f0633a68779ed09f7e0f305ed
Gerrit-Change-Number: 10028
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180905/08caaa94/attachment-0001.html>