Re: [PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 14, 2015, 20:59 UTC
Message-ID: <xmqqmvtciw69.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1450121838-7069-4-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> Provide a wrapper to read(), similar to xread(), that restarts on
> EINTR but not EAGAIN (or EWOULDBLOCK). This enables the caller to
> handle polling itself, possibly polling multiple sockets or performing
> some other action.

Do you still need this (and use of this in 4/8)? The description of a4433fd4 (run-command: remove set_nonblocking(), 2015-11-05) from the previous iteration justifies the removal of set_nonblocking() this way:

    run-command: remove set_nonblocking()
    
    strbuf_read_once can also operate on blocking file descriptors if we
    are sure they are ready.  And the poll(2) we call before calling
    this ensures that this is the case.

Updated run-command in this reroll lacks set_nonblocking(), and does have the poll(2) before it calls strbuf_read_once(). Which means that xread_nonblock() introduced here and used by [4/8] would read a file descriptor that is not marked as nonblock, the read(2) in there would never error-return with EWOULDBLOCK, and would be identical to xread() updated by [2/8] in this reroll.

So it appears to me that you can lose this and call xread() in [4/8]?

Ahh, there is a difference if the file descriptor the caller feeds strbuf_read_once() happens to be marked as nonblock. read_once() wants to return without doing the poll() in such a case. Even though this series would not introduce any use of a nonblocking file descriptor, as a general API function, [4/8] must be prepared for such a future caller, hence [3/8] is needed.

OK, thanks.