From: Junio C Hamano <gitster@pobox•com>
To: Stefan Beller <sbeller@google•com>
Cc: git@vger•kernel.org, peff@peff•net, jrnieder@gmail•com,
johannes.schindelin@gmail•com, Jens.Lehmann@web•de,
ericsunshine@gmail•com, j6t@kdbg•org
Subject: Re: [PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking
Date: Mon, 14 Dec 2015 12:59:10 -0800 [thread overview]
Message-ID: <xmqqmvtciw69.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1450121838-7069-4-git-send-email-sbeller@google.com> (Stefan Beller's message of "Mon, 14 Dec 2015 11:37:13 -0800")
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.
next prev parent reply other threads:[~2015-12-14 20:59 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-14 19:37 [PATCH 0/8] Rerolling sb/submodule-parallel-fetch for the time after 2.7 Stefan Beller
2015-12-14 19:37 ` [PATCH 1/8] submodule.c: write "Fetching submodule <foo>" to stderr Stefan Beller
2015-12-14 19:37 ` [PATCH 2/8] xread: poll on non blocking fds Stefan Beller
2015-12-14 22:58 ` Eric Sunshine
2015-12-14 23:07 ` Stefan Beller
2015-12-14 23:11 ` Junio C Hamano
2015-12-14 23:14 ` Stefan Beller
2015-12-14 19:37 ` [PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking Stefan Beller
2015-12-14 20:59 ` Junio C Hamano [this message]
2015-12-14 23:03 ` Eric Sunshine
2015-12-14 23:05 ` Eric Sunshine
2015-12-14 23:15 ` Junio C Hamano
2015-12-14 23:57 ` Jeff King
2015-12-15 0:09 ` Stefan Beller
2015-12-15 0:16 ` Jeff King
2015-12-15 0:25 ` Stefan Beller
2015-12-15 1:44 ` Jeff King
2015-12-15 6:12 ` Johannes Sixt
2015-12-15 1:40 ` Junio C Hamano
2015-12-14 19:37 ` [PATCH 4/8] strbuf: add strbuf_read_once to read " Stefan Beller
2015-12-14 23:16 ` Eric Sunshine
2015-12-14 23:27 ` Stefan Beller
2015-12-14 19:37 ` [PATCH 5/8] sigchain: add command to pop all common signals Stefan Beller
2015-12-14 19:37 ` [PATCH 6/8] run-command: add an asynchronous parallel child processor Stefan Beller
2015-12-14 20:39 ` Johannes Sixt
2015-12-14 21:40 ` Stefan Beller
2015-12-14 19:37 ` [PATCH 7/8] fetch_populated_submodules: use new parallel job processing Stefan Beller
2015-12-14 19:37 ` [PATCH 8/8] submodules: allow parallel fetching, add tests and documentation Stefan Beller
2015-12-14 20:40 ` [PATCH 0/8] Rerolling sb/submodule-parallel-fetch for the time after 2.7 Johannes Sixt
2015-12-14 21:00 ` Junio C Hamano
-- strict thread matches above, loose matches on Subject: below --
2015-09-28 23:13 [PATCH 0/8] fetch submodules in parallel Stefan Beller
2015-09-28 23:14 ` [PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking Stefan Beller
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=xmqqmvtciw69.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=Jens.Lehmann@web$(echo .)de \
--cc=ericsunshine@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=j6t@kdbg$(echo .)org \
--cc=johannes.schindelin@gmail$(echo .)com \
--cc=jrnieder@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
--cc=sbeller@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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