{"thread":{"id":"2877","subject":"EAGAIN?","startedAt":"2005-12-19T10:07:27Z","lastAt":"2005-12-20T18:59:41Z","messageCount":6,"participants":["Junio C Hamano","Linus Torvalds","H. Peter Anvin","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"13809","messageId":"7vhd95h02o.fsf@assigned-by-dhcp.cox.net","threadId":"2877","inReplyTo":null,"subject":"EAGAIN?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-19T10:07:27Z","receivedAt":"2005-12-19T10:07:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I am embarrassed to ask this in public, but anyway.\n\nI was looking at \"git grep -n EAGAIN\" output and found that many\nplaces check \"errno == EAGAIN\" without checking \"errno ==\nEINTR\", both for read(2) and write(2).  I feel that EAGAIN is\nprobably not that useful (it probably would not hurt us, though)\nbecause we do not set up nonblocking ourselves, but I am\nwondering if there is particular reason to only check EAGAIN but\nnot EINTR, or if they are just cut and paste errors.\n\n\tapply.c:: read checks EAGAIN only, write checks both.\n        cat-file.c: write checks EAGAIN only.\n        copy.c:: read checks EAGAIN only, write checks both.\n        mktag.c::  read checks EAGAIN only.\n        pkt-line.c:: both read and write checks both.\n\ttar-tree.c:: write checks EAGAIN only.\n        unpack-objects.c:: both read and write checks both.\n"},{"id":"13823","messageId":"Pine.LNX.4.64.0512191142410.4827@g5.osdl.org","threadId":"2877","inReplyTo":"7vhd95h02o.fsf@assigned-by-dhcp.cox.net","subject":"Re: EAGAIN?","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-12-19T19:46:37Z","receivedAt":"2005-12-19T19:46:37Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 19 Dec 2005, Junio C Hamano wrote:\n> \n> I was looking at \"git grep -n EAGAIN\" output and found that many\n> places check \"errno == EAGAIN\" without checking \"errno ==\n> EINTR\", both for read(2) and write(2).\n\nI suspect it's mostly in my code. It's a stupid quirk of mine.\n\nYou might as well delete those thing, but yes, if you want to replace them \nwith testing both EAGAIN and EINTR, go right ahead.\n\n\t\t\tLinus\n"},{"id":"13827","messageId":"43A732C9.2060509@zytor.com","threadId":"2877","inReplyTo":"Pine.LNX.4.64.0512191142410.4827@g5.osdl.org","subject":"Re: EAGAIN?","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-12-19T22:23:05Z","receivedAt":"2005-12-19T22:23:05Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Linus Torvalds wrote:\n> \n> On Mon, 19 Dec 2005, Junio C Hamano wrote:\n> \n>>I was looking at \"git grep -n EAGAIN\" output and found that many\n>>places check \"errno == EAGAIN\" without checking \"errno ==\n>>EINTR\", both for read(2) and write(2).\n> \n> \n> I suspect it's mostly in my code. It's a stupid quirk of mine.\n> \n> You might as well delete those thing, but yes, if you want to replace them \n> with testing both EAGAIN and EINTR, go right ahead.\n> \n\nIt might be that some of those should either be replaced by fwrite/fread \nor there should be a generic wrapper (usually called xwrite/xread).\n\n\t-hpa\n"},{"id":"13830","messageId":"7vvexkpoxf.fsf_-_@assigned-by-dhcp.cox.net","threadId":"2877","inReplyTo":"43A732C9.2060509@zytor.com","subject":"[PATCH] xread/xwrite: do not worry about EINTR at calling sites.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-20T00:55:40Z","receivedAt":"2005-12-20T00:55:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"H. Peter Anvin\" <hpa@zytor.com> writes:\n\n> Linus Torvalds wrote:\n>> On Mon, 19 Dec 2005, Junio C Hamano wrote:\n>>\n>>>I was looking at \"git grep -n EAGAIN\" output and found that many\n>>>places check \"errno == EAGAIN\" without checking \"errno ==\n>>>EINTR\", both for read(2) and write(2).\n>> I suspect it's mostly in my code. It's a stupid quirk of mine.\n>> You might as well delete those thing, but yes, if you want to\n>> replace them with testing both EAGAIN and EINTR, go right ahead.\n>>\n>\n> It might be that some of those should either be replaced by fwrite/fread \n> or there should be a generic wrapper (usually called xwrite/xread).\n\nGood idea.  Something like this I suppose....\n\n-- >8 --\nWe had errno==EINTR check after read(2)/write(2) sprinkled all\nover the places, always doing continue.  Consolidate them into\nxread()/xwrite() wrapper routines.\n\nCredits for suggestion goes to HPA -- bugs are mine.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n\n---\n\n apply.c           |   23 ++++++-----------------\n cat-file.c        |    4 +---\n copy.c            |   19 +++++++------------\n csum-file.c       |    4 +---\n git-compat-util.h |   22 ++++++++++++++++++++++\n mktag.c           |    9 ++-------\n pkt-line.c        |   11 +++--------\n tar-tree.c        |    4 +---\n unpack-objects.c  |   13 +++----------\n 9 files changed, 46 insertions(+), 63 deletions(-)\n\n7c59f627268cd446fc7e1df9847dd1e2178072da\ndiff --git a/apply.c b/apply.c\nindex 1742ab2..d5e7bfd 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -84,14 +84,11 @@ static void *read_patch_file(int fd, uns\n \t\t\tbuffer = xrealloc(buffer, alloc);\n \t\t\tnr = alloc - size;\n \t\t}\n-\t\tnr = read(fd, buffer + size, nr);\n+\t\tnr = xread(fd, buffer + size, nr);\n \t\tif (!nr)\n \t\t\tbreak;\n-\t\tif (nr < 0) {\n-\t\t\tif (errno == EAGAIN)\n-\t\t\t\tcontinue;\n+\t\tif (nr < 0)\n \t\t\tdie(\"git-apply: read returned %s\", strerror(errno));\n-\t\t}\n \t\tsize += nr;\n \t}\n \t*sizep = size;\n@@ -1006,13 +1003,8 @@ static int read_old_data(struct stat *st\n \t\t\treturn error(\"unable to open %s\", path);\n \t\tgot = 0;\n \t\tfor (;;) {\n-\t\t\tint ret = read(fd, buf + got, size - got);\n-\t\t\tif (ret < 0) {\n-\t\t\t\tif (errno == EAGAIN)\n-\t\t\t\t\tcontinue;\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t\tif (!ret)\n+\t\t\tint ret = xread(fd, buf + got, size - got);\n+\t\t\tif (ret <= 0)\n \t\t\t\tbreak;\n \t\t\tgot += ret;\n \t\t}\n@@ -1600,12 +1592,9 @@ static int try_create_file(const char *p\n \tif (fd < 0)\n \t\treturn -1;\n \twhile (size) {\n-\t\tint written = write(fd, buf, size);\n-\t\tif (written < 0) {\n-\t\t\tif (errno == EINTR || errno == EAGAIN)\n-\t\t\t\tcontinue;\n+\t\tint written = xwrite(fd, buf, size);\n+\t\tif (written < 0)\n \t\t\tdie(\"writing file %s: %s\", path, strerror(errno));\n-\t\t}\n \t\tif (!written)\n \t\t\tdie(\"out of space writing file %s\", path);\n \t\tbuf += written;\ndiff --git a/cat-file.c b/cat-file.c\nindex 7594108..96d66b4 100644\n--- a/cat-file.c\n+++ b/cat-file.c\n@@ -55,10 +55,8 @@ int main(int argc, char **argv)\n \t\tdie(\"git-cat-file %s: bad file\", argv[2]);\n \n \twhile (size > 0) {\n-\t\tlong ret = write(1, buf, size);\n+\t\tlong ret = xwrite(1, buf, size);\n \t\tif (ret < 0) {\n-\t\t\tif (errno == EAGAIN)\n-\t\t\t\tcontinue;\n \t\t\t/* Ignore epipe */\n \t\t\tif (errno == EPIPE)\n \t\t\t\tbreak;\ndiff --git a/copy.c b/copy.c\nindex e1cd5d0..7100eed 100644\n--- a/copy.c\n+++ b/copy.c\n@@ -6,32 +6,27 @@ int copy_fd(int ifd, int ofd)\n \t\tint len;\n \t\tchar buffer[8192];\n \t\tchar *buf = buffer;\n-\t\tlen = read(ifd, buffer, sizeof(buffer));\n+\t\tlen = xread(ifd, buffer, sizeof(buffer));\n \t\tif (!len)\n \t\t\tbreak;\n \t\tif (len < 0) {\n \t\t\tint read_error;\n-\t\t\tif (errno == EAGAIN)\n-\t\t\t\tcontinue;\n \t\t\tread_error = errno;\n \t\t\tclose(ifd);\n \t\t\treturn error(\"copy-fd: read returned %s\",\n \t\t\t\t     strerror(read_error));\n \t\t}\n-\t\twhile (1) {\n-\t\t\tint written = write(ofd, buf, len);\n+\t\twhile (len) {\n+\t\t\tint written = xwrite(ofd, buf, len);\n \t\t\tif (written > 0) {\n \t\t\t\tbuf += written;\n \t\t\t\tlen -= written;\n-\t\t\t\tif (!len)\n-\t\t\t\t\tbreak;\n \t\t\t}\n-\t\t\tif (!written)\n+\t\t\telse if (!written)\n \t\t\t\treturn error(\"copy-fd: write returned 0\");\n-\t\t\tif (errno == EAGAIN || errno == EINTR)\n-\t\t\t\tcontinue;\n-\t\t\treturn error(\"copy-fd: write returned %s\",\n-\t\t\t\t     strerror(errno));\n+\t\t\telse\n+\t\t\t\treturn error(\"copy-fd: write returned %s\",\n+\t\t\t\t\t     strerror(errno));\n \t\t}\n \t}\n \tclose(ifd);\ndiff --git a/csum-file.c b/csum-file.c\nindex c66b9eb..5f9249a 100644\n--- a/csum-file.c\n+++ b/csum-file.c\n@@ -15,7 +15,7 @@ static int sha1flush(struct sha1file *f,\n \tvoid *buf = f->buffer;\n \n \tfor (;;) {\n-\t\tint ret = write(f->fd, buf, count);\n+\t\tint ret = xwrite(f->fd, buf, count);\n \t\tif (ret > 0) {\n \t\t\tbuf += ret;\n \t\t\tcount -= ret;\n@@ -25,8 +25,6 @@ static int sha1flush(struct sha1file *f,\n \t\t}\n \t\tif (!ret)\n \t\t\tdie(\"sha1 file '%s' write error. Out of diskspace\", f->name);\n-\t\tif (errno == EAGAIN || errno == EINTR)\n-\t\t\tcontinue;\n \t\tdie(\"sha1 file '%s' write error (%s)\", f->name, strerror(errno));\n \t}\n }\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex ead0ede..0c98c99 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -84,6 +84,28 @@ static inline void *xcalloc(size_t nmemb\n \treturn ret;\n }\n \n+static inline ssize_t xread(int fd, void *buf, size_t len)\n+{\n+\tssize_t nr;\n+\twhile (1) {\n+\t\tnr = read(fd, buf, len);\n+\t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n+\t\t\tcontinue;\n+\t\treturn nr;\n+\t}\n+}\n+\n+static inline ssize_t xwrite(int fd, const void *buf, size_t len)\n+{\n+\tssize_t nr;\n+\twhile (1) {\n+\t\tnr = write(fd, buf, len);\n+\t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n+\t\t\tcontinue;\n+\t\treturn nr;\n+\t}\n+}\n+\n /* Sane ctype - no locale, and works with signed chars */\n #undef isspace\n #undef isdigit\ndiff --git a/mktag.c b/mktag.c\nindex 97e270a..fc6a9bf 100644\n--- a/mktag.c\n+++ b/mktag.c\n@@ -116,14 +116,9 @@ int main(int argc, char **argv)\n \t// Read the signature\n \tsize = 0;\n \tfor (;;) {\n-\t\tint ret = read(0, buffer + size, MAXSIZE - size);\n-\t\tif (!ret)\n+\t\tint ret = xread(0, buffer + size, MAXSIZE - size);\n+\t\tif (ret <= 0)\n \t\t\tbreak;\n-\t\tif (ret < 0) {\n-\t\t\tif (errno == EAGAIN)\n-\t\t\t\tcontinue;\n-\t\t\tbreak;\n-\t\t}\n \t\tsize += ret;\n \t}\n \ndiff --git a/pkt-line.c b/pkt-line.c\nindex 6947304..bb3bab0 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -19,7 +19,7 @@\n static void safe_write(int fd, const void *buf, unsigned n)\n {\n \twhile (n) {\n-\t\tint ret = write(fd, buf, n);\n+\t\tint ret = xwrite(fd, buf, n);\n \t\tif (ret > 0) {\n \t\t\tbuf += ret;\n \t\t\tn -= ret;\n@@ -27,8 +27,6 @@ static void safe_write(int fd, const voi\n \t\t}\n \t\tif (!ret)\n \t\t\tdie(\"write error (disk full?)\");\n-\t\tif (errno == EAGAIN || errno == EINTR)\n-\t\t\tcontinue;\n \t\tdie(\"write error (%s)\", strerror(errno));\n \t}\n }\n@@ -68,12 +66,9 @@ static void safe_read(int fd, void *buff\n \tint n = 0;\n \n \twhile (n < size) {\n-\t\tint ret = read(fd, buffer + n, size - n);\n-\t\tif (ret < 0) {\n-\t\t\tif (errno == EINTR || errno == EAGAIN)\n-\t\t\t\tcontinue;\n+\t\tint ret = xread(fd, buffer + n, size - n);\n+\t\tif (ret < 0)\n \t\t\tdie(\"read error (%s)\", strerror(errno));\n-\t\t}\n \t\tif (!ret)\n \t\t\tdie(\"unexpected EOF\");\n \t\tn += ret;\ndiff --git a/tar-tree.c b/tar-tree.c\nindex bacb23a..96bd143 100644\n--- a/tar-tree.c\n+++ b/tar-tree.c\n@@ -34,10 +34,8 @@ struct path_prefix {\n static void reliable_write(void *buf, unsigned long size)\n {\n \twhile (size > 0) {\n-\t\tlong ret = write(1, buf, size);\n+\t\tlong ret = xwrite(1, buf, size);\n \t\tif (ret < 0) {\n-\t\t\tif (errno == EAGAIN)\n-\t\t\t\tcontinue;\n \t\t\tif (errno == EPIPE)\n \t\t\t\texit(0);\n \t\t\tdie(\"git-tar-tree: %s\", strerror(errno));\ndiff --git a/unpack-objects.c b/unpack-objects.c\nindex cfd61ae..5c5cb12 100644\n--- a/unpack-objects.c\n+++ b/unpack-objects.c\n@@ -31,12 +31,10 @@ static void * fill(int min)\n \t\toffset = 0;\n \t}\n \tdo {\n-\t\tint ret = read(0, buffer + len, sizeof(buffer) - len);\n+\t\tint ret = xread(0, buffer + len, sizeof(buffer) - len);\n \t\tif (ret <= 0) {\n \t\t\tif (!ret)\n \t\t\t\tdie(\"early EOF\");\n-\t\t\tif (errno == EAGAIN || errno == EINTR)\n-\t\t\t\tcontinue;\n \t\t\tdie(\"read error on input: %s\", strerror(errno));\n \t\t}\n \t\tlen += ret;\n@@ -299,14 +297,9 @@ int main(int argc, char **argv)\n \n \t/* Write the last part of the buffer to stdout */\n \twhile (len) {\n-\t\tint ret = write(1, buffer + offset, len);\n-\t\tif (!ret)\n-\t\t\tbreak;\n-\t\tif (ret < 0) {\n-\t\t\tif (errno == EAGAIN || errno == EINTR)\n-\t\t\t\tcontinue;\n+\t\tint ret = xwrite(1, buffer + offset, len);\n+\t\tif (ret <= 0)\n \t\t\tbreak;\n-\t\t}\n \t\tlen -= ret;\n \t\toffset += ret;\n \t}\n-- \n0.99.9.GIT\n"},{"id":"13836","messageId":"Pine.LNX.4.63.0512201136430.29711@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2877","inReplyTo":"7vvexkpoxf.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] xread/xwrite: do not worry about EINTR at calling sites.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-12-20T10:40:05Z","receivedAt":"2005-12-20T10:40:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 19 Dec 2005, Junio C Hamano wrote:\n\n> +static inline ssize_t xwrite(int fd, const void *buf, size_t len)\n> +{\n> +\tssize_t nr;\n> +\twhile (1) {\n> +\t\tnr = write(fd, buf, len);\n> +\t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n> +\t\t\tcontinue;\n> +\t\treturn nr;\n> +\t}\n> +}\n\nIn another project I'm working on, a user insisted that on Solaris 2.7, \nwrite(2) sometimes returns ENOENT when it really means \"try again\". I \ncannot verify, since I don't have Solaris 2.7.\n\nCiao,\nDscho\n"},{"id":"13840","messageId":"Pine.LNX.4.64.0512201058570.4827@g5.osdl.org","threadId":"2877","inReplyTo":"Pine.LNX.4.63.0512201136430.29711@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] xread/xwrite: do not worry about EINTR at calling sites.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-12-20T18:59:41Z","receivedAt":"2005-12-20T18:59:41Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 20 Dec 2005, Johannes Schindelin wrote:\n> \n> In another project I'm working on, a user insisted that on Solaris 2.7, \n> write(2) sometimes returns ENOENT when it really means \"try again\". I \n> cannot verify, since I don't have Solaris 2.7.\n\nThat would be either a very confused user or a terminally sick Solaris \nkernel. In either case, not worth fixing. \n\n\t\tLinus\n"}]}