mailmap: clean up read_mailmap error handling

The error handling for the read_mailmap function is odd. It returns 1 on error, rather than -1. And it treats a non-existent mailmap as an error, even though there is no reason that one needs to exist. Unless some other mailmap source loads successfully, in which case the original error is completely masked. This does not cause any bugs, however, because no caller bothers to check the return value, anyway. Let's make this a little more robust to real errors and less surprising for future callers that do check the error code: 1. Return -1 on errors. 2. Treat a missing entry (e.g., no mailmap.file given), ENOENT, or a non-existent blob (for mailmap.blob) as "no error". 3. Complain loudly when a real error (e.g., a transient I/O error, no permission to open the mailmap file, missing or corrupted blob object, etc) occurs. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Dec 12, 2012 at 06:18 UTC 938a60d64fc91fd7d646ed33fad527b724d1e534
1 file changed +21 -11
mailmap.c
+21 -11
@@ -168,10 +168,19 @@ static int read_mailmap_file(struct string_list *map, const char *filename,
168 char **repo_abbrev)
169 {
170 char buffer[1024];
171 - FILE *f = (filename == NULL ? NULL : fopen(filename, "r"));
171 + FILE *f;
172 +
173 + if (!filename)
174 + return 0;
175 +
176 + f = fopen(filename, "r");
177 + if (!f) {
178 + if (errno == ENOENT)
179 + return 0;
180 + return error("unable to open mailmap at %s: %s",
181 + filename, strerror(errno));
182 + }
183
173 - if (f == NULL)
174 - return 1;
184 while (fgets(buffer, sizeof(buffer), f) != NULL)
185 read_mailmap_line(map, buffer, repo_abbrev);
186 fclose(f);
@@ -205,15 +214,15 @@ static int read_mailmap_blob(struct string_list *map,
214 enum object_type type;
215
216 if (!name)
208 - return 1;
217 + return 0;
218 if (get_sha1(name, sha1) < 0)
210 - return 1;
219 + return 0;
220
221 buf = read_sha1_file(sha1, &type, &size);
222 if (!buf)
214 - return 1;
223 + return error("unable to read mailmap object at %s", name);
224 if (type != OBJ_BLOB)
216 - return 1;
225 + return error("mailmap is not a blob: %s", name);
226
227 read_mailmap_buf(map, buf, size, repo_abbrev);
228
@@ -223,11 +232,12 @@ static int read_mailmap_blob(struct string_list *map,
232
233 int read_mailmap(struct string_list *map, char **repo_abbrev)
234 {
235 + int err = 0;
236 map->strdup_strings = 1;
227 - /* each failure returns 1, so >2 means all calls failed */
228 - return read_mailmap_file(map, ".mailmap", repo_abbrev) +
229 - read_mailmap_blob(map, git_mailmap_blob, repo_abbrev) +
230 - read_mailmap_file(map, git_mailmap_file, repo_abbrev) > 2;
237 + err |= read_mailmap_file(map, ".mailmap", repo_abbrev);
238 + err |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);
239 + err |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);
240 + return err;
241 }
242
243 void clear_mailmap(struct string_list *map)