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>

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 88d205f858e93f6d7c8fca17803fc6fadee47750..49e7175d9aebf5a1964fd6d61d9509a022476df1 100644 (file)
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -311,10 +311,11 @@ static const char *open_pack_file(const char *pack_name)
                        output_fd = odb_mkstemp(tmp_file, sizeof(tmp_file),
                                                "pack/tmp_pack_XXXXXX");
                        pack_name = xstrdup(tmp_file);
-               } else
+               } else {
                        output_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR, 0600);
-               if (output_fd < 0)
-                       die_errno(_("unable to create '%s'"), pack_name);
+                       if (output_fd < 0)
+                               die_errno(_("unable to create '%s'"), pack_name);
+               }
                nothread_data.pack_fd = output_fd;
        } else {
                input_fd = open(pack_name, O_RDONLY);

diff --git a/cache.h b/cache.h
index fbdf7a815a1f6b56df82877d0a5b2febe5a7d095..6c01e2746833afb395c2ac4d42716289b08b95fd 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -1674,7 +1674,10 @@ extern struct packed_git *find_sha1_pack(const unsigned char *sha1,
 extern void pack_report(void);
 
 /*
- * Create a temporary file rooted in the object database directory.
+ * Create a temporary file rooted in the object database directory, or
+ * die on failure. The filename is taken from "pattern", which should have the
+ * usual "XXXXXX" trailer, and the resulting filename is written into the
+ * "template" buffer. Returns the open descriptor.
  */
 extern int odb_mkstemp(char *template, size_t limit, const char *pattern);
 

diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 9705596014c8fcf534fd0478606d478164382913..44492c346ce530ab156c128c5377245a724671d1 100644 (file)
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -517,8 +517,6 @@ void bitmap_writer_finish(struct pack_idx_entry **index,
 
        int fd = odb_mkstemp(tmp_file, sizeof(tmp_file), "pack/tmp_bitmap_XXXXXX");
 
-       if (fd < 0)
-               die_errno("unable to create '%s'", tmp_file);
        f = sha1fd(fd, tmp_file);
 
        memcpy(header.magic, BITMAP_IDX_SIGNATURE, sizeof(BITMAP_IDX_SIGNATURE));

diff --git a/pack-write.c b/pack-write.c
index c057513f12724254afa5a1550bfa9ada9deb943c..485080a31a512dea7fff58a925b5db45973fe3f1 100644 (file)
--- a/pack-write.c
+++ b/pack-write.c
@@ -77,9 +77,9 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec
                } else {
                        unlink(index_name);
                        fd = open(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
+                       if (fd < 0)
+                               die_errno("unable to create '%s'", index_name);
                }
-               if (fd < 0)
-                       die_errno("unable to create '%s'", index_name);
                f = sha1fd(fd, index_name);
        }