getdelim(3): Fix losing data on [EAGAIN]

Currently when an [EAGAIN] is encountered we return a partial result
that does not contain the delimeter.  On the next (successful) read we
were returning the next part of the line without the preceding string
from the first failed call.

Fix this by using the same mechanism as ungetc(3) does.  For the buffered
case we could simply set fp->_r and fp->_p back to their values before
sappend() is ran but for simplicity ungetc(3) is done in there as well.

Reviewed by:	kib
Differential Revision: https://reviews.freebsd.org/D31687

2 files changed, 223 insertions, 3 deletions

diff --git a/lib/libc/stdio/getdelim.c b/lib/libc/stdio/getdelim.c
index 8d8414266c78..ad1439ed071f 100644
--- a/lib/libc/stdio/getdelim.c
+++ b/lib/libc/stdio/getdelim.c
@@ -2,6 +2,7 @@
  * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
  *
  * Copyright (c) 2009 David Schultz <das@FreeBSD.org>
+ * Copyright (c) 2021 Dell EMC
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -138,10 +139,30 @@ getdelim(char ** __restrict linep, size_t * __restrict linecapp, int delim,
 	while ((endp = memchr(fp->_p, delim, fp->_r)) == NULL) {
 		if (sappend(linep, &linelen, linecapp, fp->_p, fp->_r))
 			goto error;
+		errno = 0;
 		if (__srefill(fp)) {
-			if (!__sfeof(fp))
-				goto error;
-			goto done;	/* hit EOF */
+			if (__sfeof(fp))
+				goto done;
+			if (errno == EAGAIN) {
+				/*
+				 * We need to undo a partial read that has
+				 * been placed into linep or we would otherwise
+				 * lose it on the next read.
+				 */
+				while (linelen > 0) {
+					if (__ungetc((*linep)[--linelen],
+					    fp) == EOF)
+						goto error;
+				}
+				/*
+				 * This is not strictly needed but it is
+				 * possible a consumer has worked around an
+				 * older EAGAIN bug by buffering a partial
+				 * return.
+				 */
+				(*linep)[0] = '\0';
+			}
+			goto error;
 		}
 	}
 	endp++;	/* snarf the delimiter, too */
diff --git a/lib/libc/tests/stdio/getdelim_test.c b/lib/libc/tests/stdio/getdelim_test.c
index 8dc662fcab8f..7b20b02a36f1 100644
--- a/lib/libc/tests/stdio/getdelim_test.c
+++ b/lib/libc/tests/stdio/getdelim_test.c
@@ -1,5 +1,6 @@
 /*-
  * Copyright (c) 2009 David Schultz <das@FreeBSD.org>
+ * Copyright (c) 2021 Dell EMC
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -27,6 +28,12 @@
 #include <sys/cdefs.h>
 __FBSDID("$FreeBSD$");
 
+#include <sys/param.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+
+#include <fcntl.h>
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -221,6 +228,196 @@ ATF_TC_BODY(empty_NULL_buffer, tc)
 	fclose(fp);
 }
 
+static void
+_ipc_read(int fd, char wait_c)
+{
+	char c;
+	ssize_t len;
+
+	c = 0;
+	while (c != wait_c) {
+		len = read(fd, &c, 1);
+		ATF_CHECK_MSG(len != 0,
+		    "EOF on IPC pipe while waiting. Did other side fail?");
+		ATF_CHECK_MSG(len == 1 || errno == EINTR,
+		    "read %zu bytes errno %d\n", len, errno);
+		if (len != 1 || errno != EINTR)
+			break;
+	}
+}
+
+static void
+_ipc_write(int fd, char c)
+{
+
+	while ((write(fd, &c, 1) != 1))
+		ATF_REQUIRE(errno == EINTR);
+}
+
+static void
+ipc_wait(int ipcfd[2])
+{
+
+	_ipc_read(ipcfd[0], '+');
+	/* Send ACK. */
+	_ipc_write(ipcfd[1], '-');
+}
+
+static void
+ipc_wakeup(int ipcfd[2])
+{
+
+	_ipc_write(ipcfd[1], '+');
+	/* Wait for ACK. */
+	_ipc_read(ipcfd[0], '-');
+}
+
+static void
+_nonblock_eagain(int buf_mode)
+{
+	FILE *fp;
+	const char delim = '!';
+	const char *strs[] = {
+	    "first line partial!",
+	    "second line is sent in full!",
+	    "third line is sent partially!",
+	    "last line is sent in full!",
+	};
+	char *line;
+	size_t linecap, strslen[nitems(strs)];
+	ssize_t linelen;
+	int fd_fifo, flags, i, ipcfd[2], pipedes[2], pipedes2[2], status;
+	pid_t pid;
+
+	line = NULL;
+	linecap = 0;
+	for (i = 0; i < nitems(strslen); i++)
+		strslen[i] = strlen(strs[i]);
+	ATF_REQUIRE(pipe2(pipedes, O_CLOEXEC) == 0);
+	ATF_REQUIRE(pipe2(pipedes2, O_CLOEXEC) == 0);
+
+	(void)unlink("fifo");
+	ATF_REQUIRE(mkfifo("fifo", 0666) == 0);
+	ATF_REQUIRE((pid = fork()) >= 0);
+	if (pid == 0) {
+		close(pipedes[0]);
+		ipcfd[1] = pipedes[1];
+		ipcfd[0] = pipedes2[0];
+		close(pipedes2[1]);
+
+		ATF_REQUIRE((fd_fifo = open("fifo", O_WRONLY)) != -1);
+
+		/* Partial write. */
+		ATF_REQUIRE(write(fd_fifo, strs[0], strslen[0] - 3) ==
+		    strslen[0] - 3);
+		ipc_wakeup(ipcfd);
+
+		ipc_wait(ipcfd);
+		/* Finish off the first line. */
+		ATF_REQUIRE(write(fd_fifo,
+		    &(strs[0][strslen[0] - 3]), 3) == 3);
+		/* And include the second full line and a partial 3rd line. */
+		ATF_REQUIRE(write(fd_fifo, strs[1], strslen[1]) == strslen[1]);
+		ATF_REQUIRE(write(fd_fifo, strs[2], strslen[2] - 3) ==
+		    strslen[2] - 3);
+		ipc_wakeup(ipcfd);
+
+		ipc_wait(ipcfd);
+		/* Finish the partial write and partially send the last. */
+		ATF_REQUIRE(write(fd_fifo,
+		    &(strs[2][strslen[2] - 3]), 3) == 3);
+		ATF_REQUIRE(write(fd_fifo, strs[3], strslen[3] - 3) ==
+		    strslen[3] - 3);
+		ipc_wakeup(ipcfd);
+
+		ipc_wait(ipcfd);
+		/* Finish the write */
+		ATF_REQUIRE(write(fd_fifo,
+		    &(strs[3][strslen[3] - 3]), 3) == 3);
+		ipc_wakeup(ipcfd);
+		_exit(0);
+	}
+	ipcfd[0] = pipedes[0];
+	close(pipedes[1]);
+	close(pipedes2[0]);
+	ipcfd[1] = pipedes2[1];
+
+	ATF_REQUIRE((fp = fopen("fifo", "r")) != NULL);
+	setvbuf(fp, (char *)NULL, buf_mode, 0);
+	ATF_REQUIRE((flags = fcntl(fileno(fp), F_GETFL, 0)) != -1);
+	ATF_REQUIRE(fcntl(fileno(fp), F_SETFL, flags | O_NONBLOCK) >= 0);
+
+	/* Wait until the writer completes its partial write. */
+	ipc_wait(ipcfd);
+	ATF_REQUIRE_ERRNO(EAGAIN,
+	    (linelen = getdelim(&line, &linecap, delim, fp)) == -1);
+	ATF_REQUIRE_STREQ("", line);
+	ATF_REQUIRE(ferror(fp));
+	ATF_REQUIRE(!feof(fp));
+	clearerr(fp);
+	ipc_wakeup(ipcfd);
+
+	ipc_wait(ipcfd);
+	/*
+	 * Should now have the finished first line, a full second line,
+	 * and a partial third line.
+	 */
+	ATF_CHECK(getdelim(&line, &linecap, delim, fp) == strslen[0]);
+	ATF_REQUIRE_STREQ(strs[0], line);
+	ATF_REQUIRE(getdelim(&line, &linecap, delim, fp) == strslen[1]);
+	ATF_REQUIRE_STREQ(strs[1], line);
+
+	ATF_REQUIRE_ERRNO(EAGAIN,
+	    (linelen = getdelim(&line, &linecap, delim, fp)) == -1);
+	ATF_REQUIRE_STREQ("", line);
+	ATF_REQUIRE(ferror(fp));
+	ATF_REQUIRE(!feof(fp));
+	clearerr(fp);
+	ipc_wakeup(ipcfd);
+
+	/* Wait for the partial write to be completed and another to be done. */
+	ipc_wait(ipcfd);
+	ATF_REQUIRE((linelen = getdelim(&line, &linecap, delim, fp)) != -1);
+	ATF_REQUIRE(!ferror(fp));
+	ATF_REQUIRE(!feof(fp));
+	ATF_REQUIRE_STREQ(strs[2], line);
+	ATF_REQUIRE(linelen == strslen[2]);
+
+	ATF_REQUIRE_ERRNO(EAGAIN,
+	    (linelen = getdelim(&line, &linecap, delim, fp)) == -1);
+	ATF_REQUIRE_STREQ("", line);
+	ATF_REQUIRE(ferror(fp));
+	ATF_REQUIRE(!feof(fp));
+	clearerr(fp);
+	ipc_wakeup(ipcfd);
+
+	ipc_wait(ipcfd);
+	ATF_REQUIRE((linelen = getdelim(&line, &linecap, delim, fp)) != -1);
+	ATF_REQUIRE(!ferror(fp));
+	ATF_REQUIRE(!feof(fp));
+	ATF_REQUIRE_STREQ(strs[3], line);
+	ATF_REQUIRE(linelen == strslen[3]);
+
+	ATF_REQUIRE(waitpid(pid, &status, WEXITED) != -1);
+	ATF_REQUIRE(WIFEXITED(status));
+	ATF_REQUIRE(WEXITSTATUS(status) == 0);
+}
+
+ATF_TC_WITHOUT_HEAD(nonblock_eagain_buffered);
+ATF_TC_BODY(nonblock_eagain_buffered, tc)
+{
+
+	_nonblock_eagain(_IOFBF);
+}
+
+ATF_TC_WITHOUT_HEAD(nonblock_eagain_unbuffered);
+ATF_TC_BODY(nonblock_eagain_unbuffered, tc)
+{
+
+	_nonblock_eagain(_IONBF);
+}
+
+
 ATF_TP_ADD_TCS(tp)
 {
 
@@ -230,6 +427,8 @@ ATF_TP_ADD_TCS(tp)
 	ATF_TP_ADD_TC(tp, invalid_params);
 	ATF_TP_ADD_TC(tp, nul);
 	ATF_TP_ADD_TC(tp, empty_NULL_buffer);
+	ATF_TP_ADD_TC(tp, nonblock_eagain_unbuffered);
+	ATF_TP_ADD_TC(tp, nonblock_eagain_buffered);
 
 	return (atf_no_error());
 }