do not check odb_mkstemp return value for errors

The odb_mkstemp function does not return an error; it dies on failure instead. But many of its callers compare the resulting descriptor against -1 and die themselves. Mostly this is just pointless, but it does raise a question when looking at the callers: if they show the results of the "template" buffer after a failure, what's in it? The answer is: it doesn't matter, because it cannot happen. So let's make that clear by removing the bogus error checks. In bitmap_writer_finish(), we can drop the error-handling code entirely. In the other two cases, it's shared with the open() in another code path; we can just move the error-check next to that open() call. And while we're at it, let's flesh out the function's docstring a bit to make the error behavior clear. Signed-off-by: Jeff King <peff@peff.net>

Jeff King committed Mar 28, 2017 at 15:45 UTC 892e723afd2b5696e4d75280e730bf9f1ea92329
4 files changed +10 -8
builtin/index-pack.c
+4 -3
@@ -311,10 +311,11 @@ static const char *open_pack_file(const char *pack_name)
311 output_fd = odb_mkstemp(tmp_file, sizeof(tmp_file),
312 "pack/tmp_pack_XXXXXX");
313 pack_name = xstrdup(tmp_file);
314 - } else
314 + } else {
315 output_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR, 0600);
316 - if (output_fd < 0)
317 - die_errno(_("unable to create '%s'"), pack_name);
316 + if (output_fd < 0)
317 + die_errno(_("unable to create '%s'"), pack_name);
318 + }
319 nothread_data.pack_fd = output_fd;
320 } else {
321 input_fd = open(pack_name, O_RDONLY);
cache.h
+4 -1
@@ -1674,7 +1674,10 @@ extern struct packed_git *find_sha1_pack(const unsigned char *sha1,
1674 extern void pack_report(void);
1675
1676 /*
1677 - * Create a temporary file rooted in the object database directory.
1677 + * Create a temporary file rooted in the object database directory, or
1678 + * die on failure. The filename is taken from "pattern", which should have the
1679 + * usual "XXXXXX" trailer, and the resulting filename is written into the
1680 + * "template" buffer. Returns the open descriptor.
1681 */
1682 extern int odb_mkstemp(char *template, size_t limit, const char *pattern);
1683
pack-bitmap-write.c
-2
@@ -517,8 +517,6 @@ void bitmap_writer_finish(struct pack_idx_entry **index,
517
518 int fd = odb_mkstemp(tmp_file, sizeof(tmp_file), "pack/tmp_bitmap_XXXXXX");
519
520 - if (fd < 0)
521 - die_errno("unable to create '%s'", tmp_file);
520 f = sha1fd(fd, tmp_file);
521
522 memcpy(header.magic, BITMAP_IDX_SIGNATURE, sizeof(BITMAP_IDX_SIGNATURE));
pack-write.c
+2 -2
@@ -77,9 +77,9 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec
77 } else {
78 unlink(index_name);
79 fd = open(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
80 + if (fd < 0)
81 + die_errno("unable to create '%s'", index_name);
82 }
81 - if (fd < 0)
82 - die_errno("unable to create '%s'", index_name);
83 f = sha1fd(fd, index_name);
84 }
85