git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: Simon Ruderich <simon@ruderich.org>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
	Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
	Yaroslav Halchenko <yoh@onerussian.com>
Subject: Re: [PATCH] strbuf: clear errno before calling getdelim(3)
Date: Fri, 11 Aug 2017 10:52:51 +0200	[thread overview]
Message-ID: <e8e7f028-6e23-368c-484f-9f069bae5dc8@web.de> (raw)
In-Reply-To: <20170811075059.nn6lru7uy6s6vpza@ruderich.org>

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.
> 
> So a valid implementation could still set errno on EOF and also
> on another error (where it's required to set errno).

True, especially the part that other errors are possible.  But we can't
rely on errno being set on EOF because leaving it unchanged is allowed
as well in that
 case.

> 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.

René

  reply	other threads:[~2017-08-11  8:53 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-09 17:39 fatal: Out of memory, getdelim failed under NFS mounts Yaroslav Halchenko
2017-08-10 13:27 ` René Scharfe
2017-08-10 14:43   ` Yaroslav Halchenko
2017-08-10 19:44     ` René Scharfe
2017-08-10 18:56   ` Junio C Hamano
2017-08-10 19:44     ` Yaroslav Halchenko
2017-08-10 19:58     ` René Scharfe
2017-08-10 20:05       ` Jeff King
2017-08-10 20:29         ` Yaroslav Halchenko
2017-08-10 20:56         ` [PATCH] strbuf: clear errno before calling getdelim(3) René Scharfe
2017-08-10 21:02           ` Jeff King
2017-08-10 21:35             ` Yaroslav Halchenko
2017-08-10 21:41           ` Junio C Hamano
2017-08-11  7:50           ` Simon Ruderich
2017-08-11  8:52             ` René Scharfe [this message]
2017-08-12 10:02               ` Simon Ruderich
2017-08-12 11:57                 ` René Scharfe
2017-08-12 12:21                   ` René Scharfe
2017-08-13  4:32                   ` Jeff King
2017-08-13  5:51                     ` René Scharfe
2017-08-13  5:58                       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e8e7f028-6e23-368c-484f-9f069bae5dc8@web.de \
    --to=l.s.r@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=simon@ruderich.org \
    --cc=yoh@onerussian.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).