{"thread":{"id":"16600","subject":"[PATCH 1/3] Make some of fwrite/fclose/write/close failures visible","startedAt":"2008-12-05T00:35:48Z","lastAt":"2008-12-05T02:04:51Z","messageCount":4,"participants":["Alex Riesen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"97185","messageId":"20081205003546.GA7294@blimp.localdomain","threadId":"16600","inReplyTo":null,"subject":"[PATCH 1/3] Make some of fwrite/fclose/write/close failures visible","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-12-05T00:35:48Z","receivedAt":"2008-12-05T00:35:48Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"So that full filesystem conditions or permissions problems wont go\nunnoticed.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nThis and the follow-up patches is fallout of Windows debugging\nsessions. I implemented random error handling code just to figure out\nwhere it might be going wrong. None of that code actually helped to\nfix something (and lots was just thrown away), but some, I think,\nstill makes sense.\n\nThis patch adds error handling only to fwrite/fputs where the return\nvalue was ignored and writing was definitely into a file. BTW, libc\nheaders in Ubuntu 8.10 have warn_unused_result attribute added to many\nfunctions (fwrite(3) and write(2) amongst them). This helps finding\nthe problem places.\n\n builtin-fsck.c  |    8 ++++++--\n builtin-merge.c |    6 ++++--\n rerere.c        |   46 +++++++++++++++++++++++++++++++++++-----------\n 3 files changed, 45 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin-fsck.c b/builtin-fsck.c\nindex d3f3de9..afded5e 100644\n--- a/builtin-fsck.c\n+++ b/builtin-fsck.c\n@@ -201,12 +201,16 @@ static void check_unreachable_object(struct object *obj)\n \t\t\t\tchar *buf = read_sha1_file(obj->sha1,\n \t\t\t\t\t\t&type, &size);\n \t\t\t\tif (buf) {\n-\t\t\t\t\tfwrite(buf, size, 1, f);\n+\t\t\t\t\tif (fwrite(buf, size, 1, f) != 1)\n+\t\t\t\t\t\tdie(\"Could not write %s: %s\",\n+\t\t\t\t\t\t    filename, strerror(errno));\n \t\t\t\t\tfree(buf);\n \t\t\t\t}\n \t\t\t} else\n \t\t\t\tfprintf(f, \"%s\\n\", sha1_to_hex(obj->sha1));\n-\t\t\tfclose(f);\n+\t\t\tif (fclose(f))\n+\t\t\t\tdie(\"Could not finish %s: %s\",\n+\t\t\t\t    filename, strerror(errno));\n \t\t}\n \t\treturn;\n \t}\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex 7c2b90c..cf86975 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -293,8 +293,10 @@ static void squash_message(void)\n \t\tpretty_print_commit(rev.commit_format, commit, &out, rev.abbrev,\n \t\t\tNULL, NULL, rev.date_mode, 0);\n \t}\n-\twrite(fd, out.buf, out.len);\n-\tclose(fd);\n+\tif (write(fd, out.buf, out.len) < 0)\n+\t\tdie(\"Writing SQUASH_MSG: %s\", strerror(errno));\n+\tif (close(fd))\n+\t\tdie(\"Finishing SQUASH_MSG: %s\", strerror(errno));\n \tstrbuf_release(&out);\n }\n \ndiff --git a/rerere.c b/rerere.c\nindex 02931a1..718fb52 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -70,6 +70,19 @@ static int write_rr(struct string_list *rr, int out_fd)\n \treturn 0;\n }\n \n+static void ferr_write(const void *p, size_t count, FILE *fp, int *err)\n+{\n+\tif (!count || *err)\n+\t\treturn;\n+\tif (fwrite(p, count, 1, fp) != 1)\n+\t\t*err = errno;\n+}\n+\n+static inline void ferr_puts(const char *s, FILE *fp, int *err)\n+{\n+\tferr_write(s, strlen(s), fp, err);\n+}\n+\n static int handle_file(const char *path,\n \t unsigned char *sha1, const char *output)\n {\n@@ -82,6 +95,7 @@ static int handle_file(const char *path,\n \tstruct strbuf one = STRBUF_INIT, two = STRBUF_INIT;\n \tFILE *f = fopen(path, \"r\");\n \tFILE *out = NULL;\n+\tint wrerror = 0;\n \n \tif (!f)\n \t\treturn error(\"Could not open %s\", path);\n@@ -118,11 +132,11 @@ static int handle_file(const char *path,\n \t\t\thunk_no++;\n \t\t\thunk = RR_CONTEXT;\n \t\t\tif (out) {\n-\t\t\t\tfputs(\"<<<<<<<\\n\", out);\n-\t\t\t\tfwrite(one.buf, one.len, 1, out);\n-\t\t\t\tfputs(\"=======\\n\", out);\n-\t\t\t\tfwrite(two.buf, two.len, 1, out);\n-\t\t\t\tfputs(\">>>>>>>\\n\", out);\n+\t\t\t\tferr_puts(\"<<<<<<<\\n\", out, &wrerror);\n+\t\t\t\tferr_write(one.buf, one.len, out, &wrerror);\n+\t\t\t\tferr_puts(\"=======\\n\", out, &wrerror);\n+\t\t\t\tferr_write(two.buf, two.len, out, &wrerror);\n+\t\t\t\tferr_puts(\">>>>>>>\\n\", out, &wrerror);\n \t\t\t}\n \t\t\tif (sha1) {\n \t\t\t\tgit_SHA1_Update(&ctx, one.buf ? one.buf : \"\",\n@@ -139,7 +153,7 @@ static int handle_file(const char *path,\n \t\telse if (hunk == RR_SIDE_2)\n \t\t\tstrbuf_addstr(&two, buf);\n \t\telse if (out)\n-\t\t\tfputs(buf, out);\n+\t\t\tferr_puts(buf, out, &wrerror);\n \t\tcontinue;\n \tbad:\n \t\thunk = 99; /* force error exit */\n@@ -149,8 +163,12 @@ static int handle_file(const char *path,\n \tstrbuf_release(&two);\n \n \tfclose(f);\n-\tif (out)\n-\t\tfclose(out);\n+\tif (wrerror)\n+\t\terror(\"There were errors while writing %s (%s)\",\n+\t\t      path, strerror(wrerror));\n+\tif (out && fclose(out))\n+\t\twrerror = error(\"Failed to flush %s: %s\",\n+\t\t\t\tpath, strerror(errno));\n \tif (sha1)\n \t\tgit_SHA1_Final(sha1, &ctx);\n \tif (hunk != RR_CONTEXT) {\n@@ -158,6 +176,8 @@ static int handle_file(const char *path,\n \t\t\tunlink(output);\n \t\treturn error(\"Could not parse conflict hunks in %s\", path);\n \t}\n+\tif (wrerror)\n+\t\treturn -1;\n \treturn hunk_no;\n }\n \n@@ -200,9 +220,13 @@ static int merge(const char *name, const char *path)\n \tif (!ret) {\n \t\tFILE *f = fopen(path, \"w\");\n \t\tif (!f)\n-\t\t\treturn error(\"Could not write to %s\", path);\n-\t\tfwrite(result.ptr, result.size, 1, f);\n-\t\tfclose(f);\n+\t\t\treturn error(\"Could not open %s: %s\", path,\n+\t\t\t\t     strerror(errno));\n+\t\tif (fwrite(result.ptr, result.size, 1, f) != 1)\n+\t\t\terror(\"Could not write %s: %s\", path, strerror(errno));\n+\t\tif (fclose(f))\n+\t\t\treturn error(\"Writing %s failed: %s\", path,\n+\t\t\t\t     strerror(errno));\n \t}\n \n \tfree(cur.ptr);\n-- \n1.6.1.rc1.29.gb140\n"},{"id":"97186","messageId":"20081205003646.GB7294@blimp.localdomain","threadId":"16600","inReplyTo":"20081205003546.GA7294@blimp.localdomain","subject":"[PATCH 2/3] Make chdir failures visible","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-12-05T00:36:46Z","receivedAt":"2008-12-05T00:36:46Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Signed-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n git.c   |    4 ++--\n setup.c |    3 ++-\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex 9e5813c..940a498 100644\n--- a/git.c\n+++ b/git.c\n@@ -195,8 +195,8 @@ static int handle_alias(int *argcp, const char ***argv)\n \t\tret = 1;\n \t}\n \n-\tif (subdir)\n-\t\tchdir(subdir);\n+\tif (subdir && chdir(subdir))\n+\t\tdie(\"Cannot change to %s: %s\", subdir, strerror(errno));\n \n \terrno = saved_errno;\n \ndiff --git a/setup.c b/setup.c\nindex 78a8041..833ced2 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -470,7 +470,8 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \t\t\t}\n \t\t\tdie(\"Not a git repository\");\n \t\t}\n-\t\tchdir(\"..\");\n+\t\tif (chdir(\"..\"))\n+\t\t\tdie(\"Cannot change to %s/..: %s\", cwd, strerror(errno));\n \t}\n \n \tinside_git_dir = 0;\n-- \n1.6.1.rc1.29.gb140\n"},{"id":"97187","messageId":"20081205003914.GC7294@blimp.localdomain","threadId":"16600","inReplyTo":"20081205003546.GA7294@blimp.localdomain","subject":"[PATCH 3/3] Report symlink failures in merge-recursive","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-12-05T00:39:14Z","receivedAt":"2008-12-05T00:39:14Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Signed-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nI don't check for unlink success in the line above, because symlink\nwill fail if unlink failed to cleanup the new path.\n\n merge-recursive.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 0e988f2..a0c804c 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -525,7 +525,8 @@ static void update_file_flags(struct merge_options *o,\n \t\t\tchar *lnk = xmemdupz(buf, size);\n \t\t\tsafe_create_leading_directories_const(path);\n \t\t\tunlink(path);\n-\t\t\tsymlink(lnk, path);\n+\t\t\tif (symlink(lnk, path))\n+\t\t\t\tdie(\"failed to symlink %s: %s\", path, strerror(errno));\n \t\t\tfree(lnk);\n \t\t} else\n \t\t\tdie(\"do not know what to do with %06o %s '%s'\",\n-- \n1.6.1.rc1.29.gb140\n"},{"id":"97191","messageId":"7v63lz2x8c.fsf@gitster.siamese.dyndns.org","threadId":"16600","inReplyTo":"20081205003546.GA7294@blimp.localdomain","subject":"Re: [PATCH 1/3] Make some of fwrite/fclose/write/close failures visible","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-05T02:04:51Z","receivedAt":"2008-12-05T02:04:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"All of them obviously look correct.  Will apply to the tip of master.\n\nThanks.\n"}]}