Improve use of lockfile API

Remove remaining double close(2)'s.  i.e. close() before
commit_locked_index() or commit_lock_file().

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-add.c b/builtin-add.c
index 5c29cc2f3f680e4dee19b040e647dd8b014cf10d..4a91e3eb118850882fbc22e8d8f37e8bbfaa7617 100644 (file)
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -259,7 +259,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
  finish:
        if (active_cache_changed) {
                if (write_cache(newfd, active_cache, active_nr) ||
-                   close(newfd) || commit_locked_index(&lock_file))
+                   commit_locked_index(&lock_file))
                        die("Unable to write new index file");
        }
 

diff --git a/builtin-apply.c b/builtin-apply.c
index d57bb6e070b838230c0cf5bd15ba0e948ca7223f..15432b6782400b556333ee4117c7daaff20412ab 100644 (file)
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2921,7 +2921,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 
        if (update_index) {
                if (write_cache(newfd, active_cache, active_nr) ||
-                   close(newfd) || commit_locked_index(&lock_file))
+                   commit_locked_index(&lock_file))
                        die("Unable to write new index file");
        }
 

diff --git a/builtin-checkout-index.c b/builtin-checkout-index.c
index 70d619da8d051f8d739911cfbbf8a5255787ec07..7e42024c67a2c0fda72f94935b7d5a723c73d131 100644 (file)
--- a/builtin-checkout-index.c
+++ b/builtin-checkout-index.c
@@ -246,8 +246,8 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
                 * want to update cache.
                 */
                if (state.refresh_cache) {
-                       close(newfd); newfd = -1;
                        rollback_lock_file(&lock_file);
+                       newfd = -1;
                }
                state.refresh_cache = 0;
        }
@@ -297,7 +297,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 
        if (0 <= newfd &&
            (write_cache(newfd, active_cache, active_nr) ||
-            close(newfd) || commit_locked_index(&lock_file)))
+            commit_locked_index(&lock_file)))
                die("Unable to write new index file");
        return 0;
 }

diff --git a/builtin-commit.c b/builtin-commit.c
index 49541c0583803e4a5ec1503c02d30147933a7a36..02279360f78f3533bcc3b9ab7caae866cb7ee97b 100644 (file)
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -237,7 +237,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
                int fd = hold_locked_index(&index_lock, 1);
                add_files_to_cache(0, also ? prefix : NULL, pathspec);
                refresh_cache(REFRESH_QUIET);
-               if (write_cache(fd, active_cache, active_nr))
+               if (write_cache(fd, active_cache, active_nr) ||
+                   close_lock_file(&index_lock))
                        die("unable to write new_index file");
                commit_style = COMMIT_NORMAL;
                return index_lock.filename;
@@ -298,7 +299,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
        fd = hold_locked_index(&index_lock, 1);
        add_remove_files(&partial);
        refresh_cache(REFRESH_QUIET);
-       if (write_cache(fd, active_cache, active_nr))
+       if (write_cache(fd, active_cache, active_nr) ||
+           close_lock_file(&index_lock))
                die("unable to write new_index file");
 
        fd = hold_lock_file_for_update(&false_lock,
@@ -308,7 +310,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
        add_remove_files(&partial);
        refresh_cache(REFRESH_QUIET);
 
-       if (write_cache(fd, active_cache, active_nr))
+       if (write_cache(fd, active_cache, active_nr) ||
+           close_lock_file(&false_lock))
                die("unable to write temporary index file");
        return false_lock.filename;
 }

diff --git a/builtin-diff.c b/builtin-diff.c
index 29365a0b17348982ea806add83f3c95bea43d4f6..8d7a5697f2b429f6240a89e28422f48ef18aee3b 100644 (file)
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -190,7 +190,7 @@ static void refresh_index_quietly(void)
        refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
 
        if (active_cache_changed &&
-           !write_cache(fd, active_cache, active_nr) && !close(fd))
+           !write_cache(fd, active_cache, active_nr))
                commit_locked_index(lock_file);
 
        rollback_lock_file(lock_file);

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 807fa93b53ad16552b7d869470f2ab59a325808a..e68e01592d044a2c2570f096668856eef5caa1a2 100644 (file)
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -783,7 +783,6 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
                        unlink(shallow);
                        rollback_lock_file(&lock);
                } else {
-                       close(fd);
                        commit_lock_file(&lock);
                }
        }

diff --git a/builtin-mv.c b/builtin-mv.c
index a3f9ad174492827e42f2854287fab19915d8ef76..990e21355d189096bdc0c808fb536b7bfb85b0a1 100644 (file)
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -264,7 +264,6 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 
                if (active_cache_changed) {
                        if (write_cache(newfd, active_cache, active_nr) ||
-                           close(newfd) ||
                            commit_locked_index(&lock_file))
                                die("Unable to write new index file");
                }

diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 1923fb1914c910457d2a987d73915db365fef68d..1aaa76dd1fe42f56e25dac6c3ca0e787eb7b005e 100644 (file)
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -108,6 +108,12 @@ static int pack_refs(unsigned int flags)
                die("failed to write ref-pack file");
        if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))
                die("failed to write ref-pack file (%s)", strerror(errno));
+       /*
+        * Since the lock file was fdopen()'ed and then fclose()'ed above,
+        * assign -1 to the lock file descriptor so that commit_lock_file()
+        * won't try to close() it.
+        */
+       packed.fd = -1;
        if (commit_lock_file(&packed) < 0)
                die("unable to overwrite old ref-pack file (%s)", strerror(errno));
        if (cbdata.flags & PACK_REFS_PRUNE)

diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 43cd56a3b5f969c5f8a44c674b95b53868e9d147..c0ea0342b755c6749391803d19f852182c0c512e 100644 (file)
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -283,7 +283,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
        }
 
        if (write_cache(newfd, active_cache, active_nr) ||
-           close(newfd) || commit_locked_index(&lock_file))
+           commit_locked_index(&lock_file))
                die("unable to write new index file");
        return 0;
 }

diff --git a/builtin-rerere.c b/builtin-rerere.c
index 37e62481388aecc8ffb9b5943de248f496ca9d13..a9e3ebc13725cbe8658478797d6e1d8e097e5292 100644 (file)
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -61,9 +61,9 @@ static int write_rr(struct path_list *rr, int out_fd)
                    write_in_full(out_fd, path, length) != length)
                        die("unable to write rerere record");
        }
-       if (close(out_fd) != 0)
+       if (commit_lock_file(&write_lock) != 0)
                die("unable to write rerere record");
-       return commit_lock_file(&write_lock);
+       return 0;
 }
 
 static int handle_file(const char *path,

diff --git a/builtin-reset.c b/builtin-reset.c
index 10dba60c39d3cf7dafe6d733a4d53977c285a73c..7ee811f0b8de34025a14a53d42f4fceb477a598e 100644 (file)
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -108,7 +108,6 @@ static int update_index_refresh(int fd, struct lock_file *index_lock)
                return error("Could not read index");
        result = refresh_cache(0) ? 1 : 0;
        if (write_cache(fd, active_cache, active_nr) ||
-                       close(fd) ||
                        commit_locked_index(index_lock))
                return error ("Could not refresh index");
        return result;

diff --git a/builtin-revert.c b/builtin-revert.c
index 4bf8eb2f58ac5a81a9d6538e31f2da661db4c3e9..358af537476b0b558989f27fe60627af6977635b 100644 (file)
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -371,13 +371,13 @@ static int revert_or_cherry_pick(int argc, const char **argv)
                                        i++;
                        }
                }
-               if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
+               if (commit_lock_file(&msg_file) < 0)
                        die ("Error wrapping up %s", defmsg);
                fprintf(stderr, "Automatic %s failed.%s\n",
                        me, help_msg(commit->object.sha1));
                exit(1);
        }
-       if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
+       if (commit_lock_file(&msg_file) < 0)
                die ("Error wrapping up %s", defmsg);
        fprintf(stderr, "Finished one %s.\n", me);
 

diff --git a/builtin-rm.c b/builtin-rm.c
index a3d25e6a571584c486d252c5a8e2b182f3da86e3..c0a8bb6cf5675d671a2cca50bf72d5c60f155313 100644 (file)
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -250,7 +250,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 
        if (active_cache_changed) {
                if (write_cache(newfd, active_cache, active_nr) ||
-                   close(newfd) || commit_locked_index(&lock_file))
+                   commit_locked_index(&lock_file))
                        die("Unable to write new index file");
        }
 

diff --git a/builtin-update-index.c b/builtin-update-index.c
index e1a938d8971f11e1a1e963913ab23ff6acc0cea9..c3a14c74ed00db592db55a018db37657148a9900 100644 (file)
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -738,7 +738,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
                            get_index_file(), strerror(lock_error));
                }
                if (write_cache(newfd, active_cache, active_nr) ||
-                   close(newfd) || commit_locked_index(lock_file))
+                   commit_locked_index(lock_file))
                        die("Unable to write new index file");
        }
 

diff --git a/builtin-write-tree.c b/builtin-write-tree.c
index b89d02efec813d33f8540d2a6d433a94ce1e0a31..d16b9ed0098fd5c12fe83638f51cc3f10fd87ed7 100644 (file)
--- a/builtin-write-tree.c
+++ b/builtin-write-tree.c
@@ -35,11 +35,9 @@ int write_tree(unsigned char *sha1, int missing_ok, const char *prefix)
                                      missing_ok, 0) < 0)
                        die("git-write-tree: error building trees");
                if (0 <= newfd) {
-                       if (!write_cache(newfd, active_cache, active_nr)
-                                       && !close(newfd)) {
-                               commit_lock_file(lock_file);
+                       if (!write_cache(newfd, active_cache, active_nr) &&
+                           !commit_lock_file(lock_file))
                                newfd = -1;
-                       }
                }
                /* Not being able to write is fine -- we are only interested
                 * in updating the cache-tree part, and if the next caller
@@ -60,8 +58,7 @@ int write_tree(unsigned char *sha1, int missing_ok, const char *prefix)
                hashcpy(sha1, active_cache_tree->sha1);
 
        if (0 <= newfd)
-               close(newfd);
-       rollback_lock_file(lock_file);
+               rollback_lock_file(lock_file);
 
        return 0;
 }

diff --git a/bundle.c b/bundle.c
index 316aa7420d8bd647e304d21c6ca4a9b9cbe5548a..5c95eca07d19a6d3ed4e9000b5bb47eef90ff66e 100644 (file)
--- a/bundle.c
+++ b/bundle.c
@@ -317,6 +317,14 @@ int create_bundle(struct bundle_header *header, const char *path,
        rls.git_cmd = 1;
        if (start_command(&rls))
                return error("Could not spawn pack-objects");
+
+       /*
+        * start_command closed bundle_fd if it was > 1
+        * so set the lock fd to -1 so commit_lock_file()
+        * won't fail trying to close it.
+        */
+       lock.fd = -1;
+
        for (i = 0; i < revs.pending.nr; i++) {
                struct object *object = revs.pending.objects[i].item;
                if (object->flags & UNINTERESTING)
@@ -326,10 +334,8 @@ int create_bundle(struct bundle_header *header, const char *path,
        }
        if (finish_command(&rls))
                return error ("pack-objects died");
-       close(bundle_fd);
-       if (!bundle_to_stdout)
-               commit_lock_file(&lock);
-       return 0;
+
+       return bundle_to_stdout ? close(bundle_fd) : commit_lock_file(&lock);
 }
 
 int unbundle(struct bundle_header *header, int bundle_fd)

diff --git a/config.c b/config.c
index 857deb6c85091057f663d2eb7143f3717bd82687..526a3f4294fdce7e69f495df039cfcbfbb5e9955 100644 (file)
--- a/config.c
+++ b/config.c
@@ -955,14 +955,12 @@ int git_config_set_multivar(const char* key, const char* value,
                munmap(contents, contents_sz);
        }
 
-       if (close(fd) || commit_lock_file(lock) < 0) {
+       if (commit_lock_file(lock) < 0) {
                fprintf(stderr, "Cannot commit config file!\n");
                ret = 4;
                goto out_free;
        }
 
-       /* fd is closed, so don't try to close it below. */
-       fd = -1;
        /*
         * lock is committed, so don't try to roll it back below.
         * NOTE: Since lockfile.c keeps a linked list of all created
@@ -973,8 +971,6 @@ int git_config_set_multivar(const char* key, const char* value,
        ret = 0;
 
 out_free:
-       if (0 <= fd)
-               close(fd);
        if (lock)
                rollback_lock_file(lock);
        free(config_filename);
@@ -1072,7 +1068,7 @@ int git_config_rename_section(const char *old_name, const char *new_name)
        }
        fclose(config_file);
  unlock_and_out:
-       if (close(out_fd) || commit_lock_file(lock) < 0)
+       if (commit_lock_file(lock) < 0)
                        ret = error("Cannot commit config file!");
  out:
        free(config_filename);

diff --git a/fast-import.c b/fast-import.c
index 82e916133982800420e77d1824a2714dd120550e..3609c24288dedb5b5e1b46d77ebfce6c305a434e 100644 (file)
--- a/fast-import.c
+++ b/fast-import.c
@@ -1546,10 +1546,18 @@ static void dump_marks(void)
        }
 
        dump_marks_helper(f, 0, marks);
-       fclose(f);
-       if (commit_lock_file(&mark_lock))
+       if (ferror(f) || fclose(f))
                failure |= error("Unable to write marks file %s: %s",
                        mark_file, strerror(errno));
+       /*
+        * Since the lock file was fdopen()'ed and then fclose()'ed above,
+        * assign -1 to the lock file descriptor so that commit_lock_file()
+        * won't try to close() it.
+        */
+       mark_lock.fd = -1;
+       if (commit_lock_file(&mark_lock))
+               failure |= error("Unable to write commit file %s: %s",
+                       mark_file, strerror(errno));
 }
 
 static int read_next_command(void)

diff --git a/merge-recursive.c b/merge-recursive.c
index b34177d20f1cbbaefd232f070e1f72c677f4eafe..c292a77a81d137a3ee73111162c9aa515c975d70 100644 (file)
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1753,7 +1753,7 @@ int main(int argc, char *argv[])
 
        if (active_cache_changed &&
            (write_cache(index_fd, active_cache, active_nr) ||
-            close(index_fd) || commit_locked_index(lock)))
+            commit_locked_index(lock)))
                        die ("unable to write %s", get_index_file());
 
        return clean ? 0: 1;

diff --git a/refs.c b/refs.c
index c3ffe0393b3c5b884422d3f9389b6c89ab6fd0df..8a42d39fd7ad11948aaa7f84cbf0b1c8fc4d4a9d 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -864,7 +864,6 @@ static int repack_without_ref(const char *refname)
                        die("too long a refname '%s'", list->name);
                write_or_die(fd, line, len);
        }
-       close(fd);
        return commit_lock_file(&packlock);
 }
 
@@ -1021,12 +1020,9 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 
 void unlock_ref(struct ref_lock *lock)
 {
-       if (lock->lock_fd >= 0) {
-               close(lock->lock_fd);
-               /* Do not free lock->lk -- atexit() still looks at them */
-               if (lock->lk)
-                       rollback_lock_file(lock->lk);
-       }
+       /* Do not free lock->lk -- atexit() still looks at them */
+       if (lock->lk)
+               rollback_lock_file(lock->lk);
        free(lock->ref_name);
        free(lock->orig_ref_name);
        free(lock);
@@ -1151,7 +1147,7 @@ int write_ref_sha1(struct ref_lock *lock,
        }
        if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
            write_in_full(lock->lock_fd, &term, 1) != 1
-               || close(lock->lock_fd) < 0) {
+               || close_lock_file(lock->lk) < 0) {
                error("Couldn't write %s", lock->lk->filename);
                unlock_ref(lock);
                return -1;