wrapper.c: add and use fopen_or_warn()

pclouds · gitster · commit e9d983f116c7 · 2017-05-26T12:33:56.000+09:00
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 &lt;pclouds@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/attr.c b/attr.c
@@ -720,16 +720,13 @@ void git_attr_set_direction(enum git_attr_direction new_direction,
 
 static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)
 {
-	FILE *fp = fopen(path, "r");
+	FILE *fp = fopen_or_warn(path, "r");
 	struct attr_stack *res;
 	char buf[2048];
 	int lineno = 0;
 
-	if (!fp) {
-		if (errno != ENOENT && errno != ENOTDIR)
-			warn_on_inaccessible(path);
+	if (!fp)
 		return NULL;
-	}
 	res = xcalloc(1, sizeof(*res));
 	while (fgets(buf, sizeof(buf), fp)) {
 		char *bufp = buf;
diff --git a/bisect.c b/bisect.c
@@ -666,7 +666,7 @@ static int is_expected_rev(const struct object_id *oid)
 	if (stat(filename, &st) || !S_ISREG(st.st_mode))
 		return 0;
 
-	fp = fopen(filename, "r");
+	fp = fopen_or_warn(filename, "r");
 	if (!fp)
 		return 0;
 
diff --git 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;
diff --git a/commit.c b/commit.c
@@ -167,7 +167,7 @@ struct commit_graft *read_graft_line(char *buf, int len)
 
 static int read_graft_file(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;
diff --git a/config.c b/config.c
@@ -1422,7 +1422,7 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)
 	int ret = -1;
 	FILE *f;
 
-	f = fopen(filename, "r");
+	f = fopen_or_warn(filename, "r");
 	if (f) {
 		flockfile(f);
 		ret = do_config_from_file(fn, CONFIG_ORIGIN_FILE, filename, filename, f, data);
diff --git a/git-compat-util.h b/git-compat-util.h
@@ -802,6 +802,7 @@ extern int xmkstemp(char *template);
 extern int xmkstemp_mode(char *template, int mode);
 extern char *xgetcwd(void);
 extern FILE *fopen_for_writing(const char *path);
+extern FILE *fopen_or_warn(const char *path, const char *mode);
 
 #define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc)))
 #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc)))
diff --git a/ident.c b/ident.c
@@ -72,12 +72,10 @@ static int add_mailname_host(struct strbuf *buf)
 	FILE *mailname;
 	struct strbuf mailnamebuf = STRBUF_INIT;
 
-	mailname = fopen("/etc/mailname", "r");
-	if (!mailname) {
-		if (errno != ENOENT)
-			warning_errno("cannot open /etc/mailname");
+	mailname = fopen_or_warn("/etc/mailname", "r");
+	if (!mailname)
 		return -1;
-	}
+
 	if (strbuf_getline(&mailnamebuf, mailname) == EOF) {
 		if (ferror(mailname))
 			warning_errno("cannot read /etc/mailname");
diff --git a/remote.c b/remote.c
@@ -251,7 +251,7 @@ static const char *skip_spaces(const char *s)
 static void read_remotes_file(struct remote *remote)
 {
 	struct strbuf buf = STRBUF_INIT;
-	FILE *f = fopen(git_path("remotes/%s", remote->name), "r");
+	FILE *f = fopen_or_warn(git_path("remotes/%s", remote->name), "r");
 
 	if (!f)
 		return;
@@ -277,7 +277,7 @@ static void read_branches_file(struct remote *remote)
 {
 	char *frag;
 	struct strbuf buf = STRBUF_INIT;
-	FILE *f = fopen(git_path("branches/%s", remote->name), "r");
+	FILE *f = fopen_or_warn(git_path("branches/%s", remote->name), "r");
 
 	if (!f)
 		return;
diff --git a/rerere.c b/rerere.c
@@ -200,7 +200,7 @@ static struct rerere_id *new_rerere_id(unsigned char *sha1)
 static void read_rr(struct string_list *rr)
 {
 	struct strbuf buf = STRBUF_INIT;
-	FILE *in = fopen(git_path_merge_rr(), "r");
+	FILE *in = fopen_or_warn(git_path_merge_rr(), "r");
 
 	if (!in)
 		return;
diff --git a/sequencer.c b/sequencer.c
@@ -897,8 +897,8 @@ static void flush_rewritten_pending(void) {
 	FILE *out;
 
 	if (strbuf_read_file(&buf, rebase_path_rewritten_pending(), 82) > 0 &&
-			!get_sha1("HEAD", newsha1) &&
-			(out = fopen(rebase_path_rewritten_list(), "a"))) {
+	    !get_sha1("HEAD", newsha1) &&
+	    (out = fopen_or_warn(rebase_path_rewritten_list(), "a"))) {
 		char *bol = buf.buf, *eol;
 
 		while (*bol) {
@@ -917,7 +917,7 @@ static void flush_rewritten_pending(void) {
 
 static void record_in_rewritten(struct object_id *oid,
 		enum todo_command next_command) {
-	FILE *out = fopen(rebase_path_rewritten_pending(), "a");
+	FILE *out = fopen_or_warn(rebase_path_rewritten_pending(), "a");
 
 	if (!out)
 		return;
@@ -1378,7 +1378,7 @@ static int read_populate_todo(struct todo_list *todo_list,
 
 	if (is_rebase_i(opts)) {
 		struct todo_list done = TODO_LIST_INIT;
-		FILE *f = fopen(rebase_path_msgtotal(), "w");
+		FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w");
 
 		if (strbuf_read_file(&done.buf, rebase_path_done(), 0) > 0 &&
 				!parse_insn_buffer(done.buf.buf, &done))
diff --git a/server-info.c b/server-info.c
@@ -133,7 +133,7 @@ static int read_pack_info_file(const char *infofile)
 	char line[1000];
 	int old_cnt = 0;
 
-	fp = fopen(infofile, "r");
+	fp = fopen_or_warn(infofile, "r");
 	if (!fp)
 		return 1; /* nonexistent is not an error. */
 
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
@@ -187,6 +187,7 @@ test_expect_success 'proper error on directory "files"' '
 	echo "Error (-1) reading configuration file a-directory." >expect &&
 	mkdir a-directory &&
 	test_expect_code 2 test-config configset_get_value foo.bar a-directory 2>output &&
+	grep "^warning:" output &&
 	grep "^Error" output >actual &&
 	test_cmp expect actual
 '
@@ -196,6 +197,7 @@ test_expect_success POSIXPERM,SANITY 'proper error on non-accessible files' '
 	test_when_finished "chmod +r .git/config" &&
 	echo "Error (-1) reading configuration file .git/config." >expect &&
 	test_expect_code 2 test-config configset_get_value foo.bar .git/config 2>output &&
+	grep "^warning:" output &&
 	grep "^Error" output >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
@@ -85,8 +85,15 @@ test_expect_success 'use branch.<name>.remote if possible' '
 '
 
 test_expect_success 'confuses pattern as remote when no remote specified' '
-	cat >exp <<-\EOF &&
-	fatal: '\''refs*master'\'' does not appear to be a git repository
+	if test_have_prereq MINGW
+	then
+		# Windows does not like asterisks in pathname
+		does_not_exist=master
+	else
+		does_not_exist="refs*master"
+	fi &&
+	cat >exp <<-EOF &&
+	fatal: '\''$does_not_exist'\'' does not appear to be a git repository
 	fatal: Could not read from remote repository.
 
 	Please make sure you have the correct access rights
@@ -98,7 +105,7 @@ test_expect_success 'confuses pattern as remote when no remote specified' '
 	# fetch <branch>.
 	# We could just as easily have used "master"; the "*" emphasizes its
 	# role as a pattern.
-	test_must_fail git ls-remote refs*master >actual 2>&1 &&
+	test_must_fail git ls-remote "$does_not_exist" >actual 2>&1 &&
 	test_i18ncmp exp actual
 '
 
diff --git a/wrapper.c b/wrapper.c
@@ -428,6 +428,17 @@ int warn_on_fopen_errors(const char *path)
 	return 0;
 }
 
+FILE *fopen_or_warn(const char *path, const char *mode)
+{
+	FILE *fp = fopen(path, mode);
+
+	if (fp)
+		return fp;
+
+	warn_on_fopen_errors(path);
+	return NULL;
+}
+
 int xmkstemp(char *template)
 {
 	int fd;
diff --git a/wt-status.c b/wt-status.c
@@ -1065,7 +1065,8 @@ static void show_am_in_progress(struct wt_status *s,
 static char *read_line_from_git_path(const char *filename)
 {
 	struct strbuf buf = STRBUF_INIT;
-	FILE *fp = fopen(git_path("%s", filename), "r");
+	FILE *fp = fopen_or_warn(git_path("%s", filename), "r");
+
 	if (!fp) {
 		strbuf_release(&buf);
 		return NULL;

Original file line number	Diff line number	Diff line change
`@@ -2071,7 +2071,7 @@ static int prepare_lines(struct scoreboard *sb)`
`2071`	`2071`	`*/`
`2072`	`2072`	`static int read_ancestry(const char *graft_file)`
`2073`	`2073`	`{`
`2074`		`- FILE *fp = fopen(graft_file, "r");`
	`2074`	`+ FILE *fp = fopen_or_warn(graft_file, "r");`
`2075`	`2075`	`struct strbuf buf = STRBUF_INIT;`
`2076`	`2076`	`if (!fp)`
`2077`	`2077`	`return -1;`
Original file line number	Diff line number	Diff line change
`@@ -167,7 +167,7 @@ struct commit_graft read_graft_line(char buf, int len)`
`167`	`167`
`168`	`168`	`static int read_graft_file(const char *graft_file)`
`169`	`169`	`{`
`170`		`- FILE *fp = fopen(graft_file, "r");`
	`170`	`+ FILE *fp = fopen_or_warn(graft_file, "r");`
`171`	`171`	`struct strbuf buf = STRBUF_INIT;`
`172`	`172`	`if (!fp)`
`173`	`173`	`return -1;`
Original file line number	Diff line number	Diff line change
`@@ -200,7 +200,7 @@ static struct rerere_id new_rerere_id(unsigned char sha1)`
`200`	`200`	`static void read_rr(struct string_list *rr)`
`201`	`201`	`{`
`202`	`202`	`struct strbuf buf = STRBUF_INIT;`
`203`		`- FILE *in = fopen(git_path_merge_rr(), "r");`
	`203`	`+ FILE *in = fopen_or_warn(git_path_merge_rr(), "r");`
`204`	`204`
`205`	`205`	`if (!in)`
`206`	`206`	`return;`