From 23a9e0712d76a63c5af1caeb816943f466f0300e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Wed, 3 May 2017 17:16:46 +0700 Subject: use xfopen() in more places MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xfopen() - provides error details - explains error on reading, or writing, or whatever operation - has l10n support - prints file name in the error Some of these are missing in the places that are replaced with xfopen(), which is a clear win. In some other places, it's just less code (not as clearly a win as the previous case but still is). The only slight regresssion is in remote-testsvn, where we don't report the file class (marks files) in the error messages anymore. But since this is a _test_ svn remote transport, I'm not too concerned. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/am.c | 8 ++------ builtin/commit.c | 5 +---- builtin/fast-export.c | 4 +--- builtin/fsck.c | 3 +-- builtin/merge.c | 4 +--- builtin/pull.c | 3 +-- 6 files changed, 7 insertions(+), 20 deletions(-) (limited to 'builtin') diff --git a/builtin/am.c b/builtin/am.c index a95dd8b4e6..f5dac7783e 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1275,12 +1275,8 @@ static int parse_mail(struct am_state *state, const char *mail) die("BUG: invalid value for state->scissors"); } - mi.input = fopen(mail, "r"); - if (!mi.input) - die("could not open input"); - mi.output = fopen(am_path(state, "info"), "w"); - if (!mi.output) - die("could not open output 'info'"); + mi.input = xfopen(mail, "r"); + mi.output = xfopen(am_path(state, "info"), "w"); if (mailinfo(&mi, am_path(state, "msg"), am_path(state, "patch"))) die("could not parse patch"); diff --git a/builtin/commit.c b/builtin/commit.c index 1d805f5da8..eda0d32311 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1695,10 +1695,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) if (!reflog_msg) reflog_msg = "commit (merge)"; pptr = commit_list_append(current_head, pptr); - fp = fopen(git_path_merge_head(), "r"); - if (fp == NULL) - die_errno(_("could not open '%s' for reading"), - git_path_merge_head()); + fp = xfopen(git_path_merge_head(), "r"); while (strbuf_getline_lf(&m, fp) != EOF) { struct commit *parent; diff --git a/builtin/fast-export.c b/builtin/fast-export.c index e0220630d0..128b99e6da 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -905,9 +905,7 @@ static void export_marks(char *file) static void import_marks(char *input_file) { char line[512]; - FILE *f = fopen(input_file, "r"); - if (!f) - die_errno("cannot read '%s'", input_file); + FILE *f = xfopen(input_file, "r"); while (fgets(line, sizeof(line), f)) { uint32_t mark; diff --git a/builtin/fsck.c b/builtin/fsck.c index b5e13a4556..00beaaa4e6 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -280,8 +280,7 @@ static void check_unreachable_object(struct object *obj) free(filename); return; } - if (!(f = fopen(filename, "w"))) - die_errno("Could not open '%s'", filename); + f = xfopen(filename, "w"); if (obj->type == OBJ_BLOB) { if (stream_blob_to_fd(fileno(f), &obj->oid, NULL, 1)) die_errno("Could not write '%s'", filename); diff --git a/builtin/merge.c b/builtin/merge.c index 703827f006..65a1501858 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -839,9 +839,7 @@ static int suggest_conflicts(void) struct strbuf msgbuf = STRBUF_INIT; filename = git_path_merge_msg(); - fp = fopen(filename, "a"); - if (!fp) - die_errno(_("Could not open '%s' for writing"), filename); + fp = xfopen(filename, "a"); append_conflicts_hint(&msgbuf); fputs(msgbuf.buf, fp); diff --git a/builtin/pull.c b/builtin/pull.c index dd1a4a94e4..589c25becf 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -337,8 +337,7 @@ static void get_merge_heads(struct oid_array *merge_heads) struct strbuf sb = STRBUF_INIT; struct object_id oid; - if (!(fp = fopen(filename, "r"))) - die_errno(_("could not open '%s' for reading"), filename); + fp = xfopen(filename, "r"); while (strbuf_getline_lf(&sb, fp) != EOF) { if (get_oid_hex(sb.buf, &oid)) continue; /* invalid line: does not start with SHA1 */ -- cgit v1.2.3 From 02912f477586befaf46e0db4f7c47b52081d5b6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Wed, 3 May 2017 17:16:47 +0700 Subject: clone: use xfopen() instead of fopen() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit copy_alternates() called fopen() without handling errors. By switching to xfopen(), this bug is fixed. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/clone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/clone.c b/builtin/clone.c index de85b85254..dde4fe73af 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -357,7 +357,7 @@ static void copy_alternates(struct strbuf *src, struct strbuf *dst, * to turn entries with paths relative to the original * absolute, so that they can be used in the new repository. */ - FILE *in = fopen(src->buf, "r"); + FILE *in = xfopen(src->buf, "r"); struct strbuf line = STRBUF_INIT; while (strbuf_getline(&line, in) != EOF) { -- cgit v1.2.3 From e9d983f116c7de43f40a49aae60ebfe107f153ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Wed, 3 May 2017 17:16:50 +0700 Subject: wrapper.c: add and use fopen_or_warn() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When fopen() returns NULL, it could be because the given path does not exist, but it could also be some other errors and the caller has to check. Add a wrapper so we don't have to repeat the same error check everywhere. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/blame.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/blame.c b/builtin/blame.c index 07506a3e45..34445d7894 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2071,7 +2071,7 @@ static int prepare_lines(struct scoreboard *sb) */ static int read_ancestry(const char *graft_file) { - FILE *fp = fopen(graft_file, "r"); + FILE *fp = fopen_or_warn(graft_file, "r"); struct strbuf buf = STRBUF_INIT; if (!fp) return -1; -- cgit v1.2.3 From 5118d7f4e6e00b7eac3ce08a16392a732edc0b2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Wed, 3 May 2017 17:16:55 +0700 Subject: print errno when reporting a system call error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/log.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/log.c b/builtin/log.c index b3b10cc1ed..26d6a3cf14 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -858,7 +858,8 @@ static int open_next_file(struct commit *commit, const char *subject, printf("%s\n", filename.buf + outdir_offset); if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL) - return error(_("Cannot open patch file %s"), filename.buf); + return error_errno(_("Cannot open patch file %s"), + filename.buf); strbuf_release(&filename); return 0; -- cgit v1.2.3 From 15d980a785f8c962bc032df40cb42fdc269c9dc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Wed, 3 May 2017 17:16:56 +0700 Subject: log: fix memory leak in open_next_file() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Noticed-by: Jeff King Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/log.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'builtin') diff --git a/builtin/log.c b/builtin/log.c index 26d6a3cf14..f075838df9 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -842,8 +842,10 @@ static int open_next_file(struct commit *commit, const char *subject, if (output_directory) { strbuf_addstr(&filename, output_directory); if (filename.len >= - PATH_MAX - FORMAT_PATCH_NAME_MAX - suffix_len) + PATH_MAX - FORMAT_PATCH_NAME_MAX - suffix_len) { + strbuf_release(&filename); return error(_("name of output directory is too long")); + } strbuf_complete(&filename, '/'); } @@ -857,9 +859,11 @@ static int open_next_file(struct commit *commit, const char *subject, if (!quiet) printf("%s\n", filename.buf + outdir_offset); - if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL) - return error_errno(_("Cannot open patch file %s"), - filename.buf); + if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL) { + error_errno(_("Cannot open patch file %s"), filename.buf); + strbuf_release(&filename); + return -1; + } strbuf_release(&filename); return 0; -- cgit v1.2.3