diff options
| author | Jeff King <peff@peff.net> | 2017-03-28 15:45:25 -0400 |
|---|---|---|
| committer | Junio C Hamano <gitster@pobox.com> | 2017-03-28 15:28:04 -0700 |
| commit | 892e723afd2b5696e4d75280e730bf9f1ea92329 (patch) | |
| tree | 072a0e33a4353dffa1e54bcdc58b9f80658cc2c1 | |
| parent | e1104a5ee539408b81566066aaa6963cb87d5cd6 (diff) | |
| download | git-892e723afd2b5696e4d75280e730bf9f1ea92329.tar.gz | |
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>
| -rw-r--r-- | builtin/index-pack.c | 7 | ||||
| -rw-r--r-- | cache.h | 5 | ||||
| -rw-r--r-- | pack-bitmap-write.c | 2 | ||||
| -rw-r--r-- | pack-write.c | 4 |
4 files changed, 10 insertions, 8 deletions
diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 88d205f858..49e7175d9a 100644 --- 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); @@ -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 9705596014..44492c346c 100644 --- 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 c057513f12..485080a31a 100644 --- 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); } |