git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org
Subject: [PATCH 3/2] hoist out handle_nonblock function for xread and xwrite
Date: Sun, 10 Jul 2016 08:20:46 +0000	[thread overview]
Message-ID: <20160710082046.GB29312@whir> (raw)
In-Reply-To: <20160710044529.GA10777@sigill.intra.peff.net>

At least for me, this improves the readability of xread and
xwrite; hopefully allowing missing "continue" statements to
be spotted more easily.

Signed-off-by: Eric Wong <e@80x24.org>
Signed-off-by: Jeff King <peff@peff.net>
---
  Jeff King <peff@peff.net> wrote:
  > On Sun, Jul 10, 2016 at 03:47:36AM +0000, Eric Wong wrote:
  > > Yes, I'm fine with either, but I'm slightly thrown off by
  > > a function relying on errno being set by the caller, even if it
  > > is errno.  So maybe localizing it is better (see below)
  > 
  > Yeah, I had a similar reservation, but didn't want to clutter the
  > interface. However, just passing errno isn't too bad (as you showed
  > below), and is much less magical.
  > 
  > Do you want to squash that and re-send the whole patch to make Junio's
  > life easier?

  Done, also updated the subject to match the new function name.

  I wasn't attached to the "io_wait" name, either, as it could
  be confused with "I/O wait" commonly associated with disk I/O
  and not often with socket/pipe I/O that poll gets used for.

 wrapper.c | 48 ++++++++++++++++++++----------------------------
 1 file changed, 20 insertions(+), 28 deletions(-)

diff --git a/wrapper.c b/wrapper.c
index 0b920f1..78f6431 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -227,6 +227,24 @@ int xopen(const char *path, int oflag, ...)
 	}
 }
 
+static int handle_nonblock(int fd, short poll_events, int err)
+{
+	struct pollfd pfd;
+
+	if (err != EAGAIN && err != EWOULDBLOCK)
+		return 0;
+
+	pfd.fd = fd;
+	pfd.events = poll_events;
+
+	/*
+	 * no need to check for errors, here;
+	 * a subsequent read/write will detect unrecoverable errors
+	 */
+	poll(&pfd, 1, -1);
+	return 1;
+}
+
 /*
  * xread() is the same a read(), but it automatically restarts read()
  * operations with a recoverable error (EAGAIN and EINTR). xread()
@@ -242,21 +260,8 @@ ssize_t xread(int fd, void *buf, size_t len)
 		if (nr < 0) {
 			if (errno == EINTR)
 				continue;
-			if (errno == EAGAIN || errno == EWOULDBLOCK) {
-				struct pollfd pfd;
-				pfd.events = POLLIN;
-				pfd.fd = fd;
-				/*
-				 * it is OK if this poll() failed; we
-				 * want to leave this infinite loop
-				 * only when read() returns with
-				 * success, or an expected failure,
-				 * which would be checked by the next
-				 * call to read(2).
-				 */
-				poll(&pfd, 1, -1);
+			if (handle_nonblock(fd, POLLIN, errno))
 				continue;
-			}
 		}
 		return nr;
 	}
@@ -277,21 +282,8 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
 		if (nr < 0) {
 			if (errno == EINTR)
 				continue;
-			if (errno == EAGAIN || errno == EWOULDBLOCK) {
-				struct pollfd pfd;
-				pfd.events = POLLOUT;
-				pfd.fd = fd;
-				/*
-				 * it is OK if this poll() failed; we
-				 * want to leave this infinite loop
-				 * only when write() returns with
-				 * success, or an expected failure,
-				 * which would be checked by the next
-				 * call to write(2).
-				 */
-				poll(&pfd, 1, -1);
+			if (handle_nonblock(fd, POLLOUT, errno))
 				continue;
-			}
 		}
 
 		return nr;
-- 
EW

  reply	other threads:[~2016-07-10  8:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-08 22:59 What's cooking in git.git (Jul 2016, #03; Fri, 8) Junio C Hamano
2016-07-09 23:25 ` Eric Wong
2016-07-11 17:51   ` Junio C Hamano
2016-07-09 23:45 ` Eric Wong
2016-07-10  2:52   ` Jeff King
2016-07-10  3:47     ` Eric Wong
2016-07-10  4:45       ` Jeff King
2016-07-10  8:20         ` Eric Wong [this message]
2016-07-11  6:24 ` Johannes Schindelin
2016-07-11 17:13   ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20160710082046.GB29312@whir \
    --to=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).