Re: [PATCH] strbuf: clear errno before calling getdelim(3)

From

Simon Ruderich <simon@ruderich.org>

Date

Aug 12, 2017, 10:02 UTC

Message-ID

<20170812100252.bhbgg5jjlom7dfl2@ruderich.org>

In-Reply-To

<e8e7f028-6e23-368c-484f-9f069bae5dc8@web.de>

On Fri, Aug 11, 2017 at 10:52:51AM +0200, René Scharfe wrote:

> Am 11.08.2017 um 09:50 schrieb Simon Ruderich:
>> On Thu, Aug 10, 2017 at 10:56:40PM +0200, René Scharfe wrote:
>>> getdelim(3) returns -1 at the end of the file and if it encounters an
>>> error, but sets errno only in the latter case.  Set errno to zero before
>>> calling it to avoid misdiagnosing an out-of-memory condition due to a
>>> left-over value from some other function call.
>>
>> getdelim(3p) doesn't explicitly forbid changing the errno on EOF:
>>
>>      If no characters were read, and the end-of-file indicator for
>>      the stream is set, or if the stream is at end-of-file, the
>>      end-of-file indicator for the stream shall be set and the
>>      function shall return −1. If an error occurs, the error
>>      indicator for the stream shall be set, and the function shall
>>      return −1 and set errno to indicate the error.
>
> [snip]
>
>> I don't think that it matters in practice, but the "most" correct
>> way to handle this would be to check if feof(3) is true to check
>> for the non-errno case.
>
> Only if errors at EOF are guaranteed to be impossible.  Imagine
> getdelim being able to read to the end and then failing to get memory
> for the final NUL.  Other scenarios may be possible.

For example like this (as replacement for the original patch):

diff --git i/strbuf.c w/strbuf.c
index 89d22e3b0..831be21ce 100644
--- i/strbuf.c
+++ w/strbuf.c
@@ -495,7 +495,7 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
 	 * memory and retry, but that's unlikely to help for a malloc small
 	 * enough to hold a single line of input, anyway.
 	 */
-	if (errno == ENOMEM)
+	if (ferror(fp) && errno == ENOMEM)
 		die("Out of memory, getdelim failed");
 
 	/*

An edge case is if the error indicator was already set before
calling getdelim() and the errno was already set to ENOMEM. This
could be fixed by checking ferror() before calling getdelim, but
I'm not sure if that's necessary:

diff --git i/strbuf.c w/strbuf.c
index 89d22e3b0..4d394bb91 100644
--- i/strbuf.c
+++ w/strbuf.c
@@ -468,7 +468,7 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
 {
 	ssize_t r;
 
-	if (feof(fp))
+	if (feof(fp) || ferror(fp))
 		return EOF;
 
 	strbuf_reset(sb);

Regards
Simon

-- 
+ Privatsphäre ist notwendig
+ Ich verwende GnuPG http://gnupg.org
+ Öffentlicher Schlüssel: 0x92FEFDB7E44C32F9