From owner-dev-commits-src-main@freebsd.org Thu Sep 2 18:26:29 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id DC1C4663F01; Thu, 2 Sep 2021 18:26:29 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4H0qBY5Y15z3wDV; Thu, 2 Sep 2021 18:26:29 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id A67591DE86; Thu, 2 Sep 2021 18:26:29 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 182IQTRU070055; Thu, 2 Sep 2021 18:26:29 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 182IQTg4070054; Thu, 2 Sep 2021 18:26:29 GMT (envelope-from git) Date: Thu, 2 Sep 2021 18:26:29 GMT Message-Id: <202109021826.182IQTg4070054@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Bryan Drewery Subject: git: 8f8a794775bd - main - getdelim(3): Fix losing data on [EAGAIN] MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: bdrewery X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 8f8a794775bd6da69514d008fe03edb808bbc67d Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Sep 2021 18:26:29 -0000 The branch main has been updated by bdrewery: URL: https://cgit.FreeBSD.org/src/commit/?id=8f8a794775bd6da69514d008fe03edb808bbc67d commit 8f8a794775bd6da69514d008fe03edb808bbc67d Author: Bryan Drewery AuthorDate: 2021-08-25 18:37:11 +0000 Commit: Bryan Drewery CommitDate: 2021-09-02 18:26:26 +0000 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 --- lib/libc/stdio/getdelim.c | 27 ++++- lib/libc/tests/stdio/getdelim_test.c | 199 +++++++++++++++++++++++++++++++++++ 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 + * 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 + * Copyright (c) 2021 Dell EMC * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -27,6 +28,12 @@ #include __FBSDID("$FreeBSD$"); +#include +#include +#include +#include + +#include #include #include #include @@ -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()); }