From 1f6cbdb434114139081fe65a9bafe775e9ab6c41 Mon Sep 17 00:00:00 2001 From: Rich Felker Date: Sun, 16 Sep 2018 22:42:59 -0400 Subject: [PATCH] getdelim: only grow buffer when necessary, improve OOM behavior commit b114190b29417fff6f701eea3a3b3b6030338280 introduced spurious realloc of the output buffer in cases where the result would exactly fit in the caller-provided buffer. this is contrary to a strict reading of the spec, which only allows realloc when the provided buffer is "of insufficient size". revert the adjustment of the realloc threshold, and instead push the byte read by getc_unlocked (for which the adjustment was made) back into the stdio buffer if it does not fit in the output buffer, to be read in the next loop iteration. in order not to leave a pushed-back byte in the stdio buffer if realloc fails (which would violate the invariant that logical FILE position and underlying open file description offset match for unbuffered FILEs), the OOM code path must be changed. it would suffice move just one byte in this case, but from a QoI perspective, in the event of ENOMEM the entire output buffer (up to the allocated length reported via *n) should contain bytes read from the FILE stream. otherwise the caller has no way to distinguish trunated data from uninitialized buffer space. the SIZE_MAX/2 check is removed since the sum of disjoint object sizes is assumed not to be able to overflow, leaving just one OOM code path. --- src/stdio/getdelim.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/stdio/getdelim.c b/src/stdio/getdelim.c index c313775d..d2f5b15a 100644 --- a/src/stdio/getdelim.c +++ b/src/stdio/getdelim.c @@ -32,15 +32,25 @@ ssize_t getdelim(char **restrict s, size_t *restrict n, int delim, FILE *restric z = 0; k = 0; } - if (i+k+1 >= *n) { - if (k >= SIZE_MAX/2-i) goto oom; + if (i+k >= *n) { size_t m = i+k+2; if (!z && m < SIZE_MAX/4) m += m/2; tmp = realloc(*s, m); if (!tmp) { m = i+k+2; tmp = realloc(*s, m); - if (!tmp) goto oom; + if (!tmp) { + /* Copy as much as fits and ensure no + * pushback remains in the FILE buf. */ + k = *n-i; + memcpy(*s+i, f->rpos, k); + f->rpos += k; + f->mode |= f->mode-1; + f->flags |= F_ERR; + FUNLOCK(f); + errno = ENOMEM; + return -1; + } } *s = tmp; *n = m; @@ -56,19 +66,16 @@ ssize_t getdelim(char **restrict s, size_t *restrict n, int delim, FILE *restric } break; } - if (((*s)[i++] = c) == delim) break; + /* If the byte read by getc won't fit without growing the + * output buffer, push it back for next iteration. */ + if (i+1 >= *n) *--f->rpos = c; + else if (((*s)[i++] = c) == delim) break; } (*s)[i] = 0; FUNLOCK(f); return i; -oom: - f->mode |= f->mode-1; - f->flags |= F_ERR; - FUNLOCK(f); - errno = ENOMEM; - return -1; } weak_alias(getdelim, __getdelim); -- 2.25.1