replace direct calls to unlink(2) with unlink_or_warn

This helps to notice when something's going wrong, especially on
systems which lock open files.

I used the following criteria when selecting the code for replacement:
- it was already printing a warning for the unlink failures
- it is in a function which already printing something or is
  called from such a function
- it is in a static function, returning void and the function is only
  called from a builtin main function (cmd_)
- it is in a function which handles emergency exit (signal handlers)
- it is in a function which is obvously cleaning up the lockfiles

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-apply.c b/builtin-apply.c
index 7b404ef6601e5b7820e9b1b6812225639e3ea4b1..8a3771e87e1ef2ac7a1ee70133b8206f0f18cbb5 100644 (file)
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2781,7 +2781,7 @@ static void remove_file(struct patch *patch, int rmdir_empty)
                        if (rmdir(patch->old_name))
                                warning("unable to remove submodule %s",
                                        patch->old_name);
-               } else if (!unlink(patch->old_name) && rmdir_empty) {
+               } else if (!unlink_or_warn(patch->old_name) && rmdir_empty) {
                        remove_path(patch->old_name);
                }
        }
@@ -2891,7 +2891,7 @@ static void create_one_file(char *path, unsigned mode, const char *buf, unsigned
                        if (!try_create_file(newpath, mode, buf, size)) {
                                if (!rename(newpath, path))
                                        return;
-                               unlink(newpath);
+                               unlink_or_warn(newpath);
                                break;
                        }
                        if (errno != EEXIST)

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 5d134be47c85019dd66aed742eb845f6245cbf4d..bd97cfd9bfdbc6bd25e5364ba84dad44059d2ac4 100644 (file)
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -814,7 +814,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
                fd = hold_lock_file_for_update(&lock, shallow,
                                               LOCK_DIE_ON_ERROR);
                if (!write_shallow_commits(fd, 0)) {
-                       unlink(shallow);
+                       unlink_or_warn(shallow);
                        rollback_lock_file(&lock);
                } else {
                        commit_lock_file(&lock);

diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c
index 4942892e9f83abc7beed6570d508cfe76da126a0..00590b1c3c2cceda7a75537c8680a96cd2ef84d4 100644 (file)
--- a/builtin-prune-packed.c
+++ b/builtin-prune-packed.c
@@ -28,8 +28,8 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts)
                memcpy(pathname + len, de->d_name, 38);
                if (opts & DRY_RUN)
                        printf("rm -f %s\n", pathname);
-               else if (unlink(pathname) < 0)
-                       error("unable to unlink %s", pathname);
+               else
+                       unlink_or_warn(pathname);
                display_progress(progress, i + 1);
        }
        pathname[len] = 0;

diff --git a/builtin-prune.c b/builtin-prune.c
index 545e9c1f9458ed936e2a99a208f75b70f368294c..145ba83651e9c8560e20d6ab449ce64783bc65f5 100644 (file)
--- a/builtin-prune.c
+++ b/builtin-prune.c
@@ -27,7 +27,7 @@ static int prune_tmp_object(const char *path, const char *filename)
        }
        printf("Removing stale temporary file %s\n", fullpath);
        if (!show_only)
-               unlink(fullpath);
+               unlink_or_warn(fullpath);
        return 0;
 }
 
@@ -47,7 +47,7 @@ static int prune_object(char *path, const char *filename, const unsigned char *s
                       (type > 0) ? typename(type) : "unknown");
        }
        if (!show_only)
-               unlink(fullpath);
+               unlink_or_warn(fullpath);
        return 0;
 }
 

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index a970b39505b5dda0bfa32a1a9c14ad676d28ec92..035b723e50a72427a26102b91f5900b131d44095 100644 (file)
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -702,7 +702,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
                        unpack_status = unpack();
                execute_commands(unpack_status);
                if (pack_lockfile)
-                       unlink(pack_lockfile);
+                       unlink_or_warn(pack_lockfile);
                if (report_status)
                        report(unpack_status);
                run_receive_hook(post_receive_hook);

diff --git a/builtin-remote.c b/builtin-remote.c
index 2ed752cbf1803c243145ecc73d6abbf758a2203a..71abf68404f5b260ba96208717d89e50e778dd36 100644 (file)
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -525,8 +525,8 @@ static int migrate_file(struct remote *remote)
                path = git_path("remotes/%s", remote->name);
        else if (remote->origin == REMOTE_BRANCHES)
                path = git_path("branches/%s", remote->name);
-       if (path && unlink(path))
-               warning("failed to remove '%s'", path);
+       if (path)
+               unlink_or_warn(path);
        return 0;
 }
 

diff --git a/builtin-rerere.c b/builtin-rerere.c
index 020af7377bb9aa1bced7489d40169e3cb2ff7682..adfb7b5f48597c19c23235b74cf1a0c779985dc6 100644 (file)
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -116,7 +116,7 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
                        if (!has_rerere_resolution(name))
                                unlink_rr_item(name);
                }
-               unlink(git_path("rr-cache/MERGE_RR"));
+               unlink_or_warn(git_path("rr-cache/MERGE_RR"));
        } else if (!strcmp(argv[1], "gc"))
                garbage_collect(&merge_rr);
        else if (!strcmp(argv[1], "status"))

diff --git a/builtin-tag.c b/builtin-tag.c
index 01e73747d02f384c5e31b846340a4b586c84aab3..e54443009420d98dc281285e997b32f2a263b996 100644 (file)
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -338,7 +338,7 @@ static void create_tag(const unsigned char *object, const char *tag,
                exit(128);
        }
        if (path) {
-               unlink(path);
+               unlink_or_warn(path);
                free(path);
        }
 }

diff --git a/builtin-verify-tag.c b/builtin-verify-tag.c
index 729a1593e61d87ad4596f07e7faedac81de64e81..7f7fda42f9b7ab272ff2b3fa4ad1984a79f11ce0 100644 (file)
--- a/builtin-verify-tag.c
+++ b/builtin-verify-tag.c
@@ -55,7 +55,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
        close(gpg.in);
        ret = finish_command(&gpg);
 
-       unlink(path);
+       unlink_or_warn(path);
 
        return ret;
 }

diff --git a/diff.c b/diff.c
index 3ac71686ebef3d3f9c8b0b72adf612e9535031a5..6802f5ac12ae7e811b7ef5cb1214d0fd78b46d53 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -189,7 +189,7 @@ static void remove_tempfile(void)
        int i;
        for (i = 0; i < ARRAY_SIZE(diff_temp); i++) {
                if (diff_temp[i].name == diff_temp[i].tmp_path)
-                       unlink(diff_temp[i].name);
+                       unlink_or_warn(diff_temp[i].name);
                diff_temp[i].name = NULL;
        }
 }

diff --git a/entry.c b/entry.c
index 915514aa5c43d3fd39f077a79f63ac27b841d255..cc841edf9051460b3a382ea25c0097f245ec8884 100644 (file)
--- a/entry.c
+++ b/entry.c
@@ -35,7 +35,7 @@ static void create_directories(const char *path, int path_len,
                 */
                if (mkdir(buf, 0777)) {
                        if (errno == EEXIST && state->force &&
-                           !unlink(buf) && !mkdir(buf, 0777))
+                           !unlink_or_warn(buf) && !mkdir(buf, 0777))
                                continue;
                        die("cannot create directory at %s", buf);
                }

diff --git a/fast-import.c b/fast-import.c
index 8d959af3b2346a03f737c47d6dd000dfd917f69d..6a618e9163a8a0f871e5aeada78502f73f2c896a 100644 (file)
--- a/fast-import.c
+++ b/fast-import.c
@@ -931,7 +931,7 @@ static void unkeep_all_packs(void)
                struct packed_git *p = all_packs[k];
                snprintf(name, sizeof(name), "%s/pack/pack-%s.keep",
                         get_object_directory(), sha1_to_hex(p->sha1));
-               unlink(name);
+               unlink_or_warn(name);
        }
 }
 
@@ -981,7 +981,7 @@ static void end_packfile(void)
        }
        else {
                close(old_p->pack_fd);
-               unlink(old_p->pack_name);
+               unlink_or_warn(old_p->pack_name);
        }
        free(old_p);
 

diff --git a/http-push.c b/http-push.c
index 5138224cc372482fb60afd350fa40e37d019f367..29e8ebfebb8947cc76b5631ae143d0003e5b3d51 100644 (file)
--- a/http-push.c
+++ b/http-push.c
@@ -315,9 +315,9 @@ static void start_fetch_loose(struct transfer_request *request)
                 "%s.temp", filename);
 
        snprintf(prevfile, sizeof(prevfile), "%s.prev", request->filename);
-       unlink(prevfile);
+       unlink_or_warn(prevfile);
        rename(request->tmpfile, prevfile);
-       unlink(request->tmpfile);
+       unlink_or_warn(request->tmpfile);
 
        if (request->local_fileno != -1)
                error("fd leakage in start: %d", request->local_fileno);
@@ -372,7 +372,7 @@ static void start_fetch_loose(struct transfer_request *request)
                } while (prev_read > 0);
                close(prevlocal);
        }
-       unlink(prevfile);
+       unlink_or_warn(prevfile);
 
        /* Reset inflate/SHA1 if there was an error reading the previous temp
           file; also rewind to the beginning of the local file. */
@@ -784,7 +784,7 @@ static void finish_request(struct transfer_request *request)
                    request->http_code != 416) {
                        if (stat(request->tmpfile, &st) == 0) {
                                if (st.st_size == 0)
-                                       unlink(request->tmpfile);
+                                       unlink_or_warn(request->tmpfile);
                        }
                } else {
                        if (request->http_code == 416)
@@ -793,9 +793,9 @@ static void finish_request(struct transfer_request *request)
                        git_inflate_end(&request->stream);
                        git_SHA1_Final(request->real_sha1, &request->c);
                        if (request->zret != Z_STREAM_END) {
-                               unlink(request->tmpfile);
+                               unlink_or_warn(request->tmpfile);
                        } else if (hashcmp(request->obj->sha1, request->real_sha1)) {
-                               unlink(request->tmpfile);
+                               unlink_or_warn(request->tmpfile);
                        } else {
                                request->rename =
                                        move_temp_to_file(

diff --git a/http-walker.c b/http-walker.c
index c5a3ea3b31045be9407579a9d1aba222ee3e9914..7321ccc9fe751a1e608c6620bcce401eac7ff98c 100644 (file)
--- a/http-walker.c
+++ b/http-walker.c
@@ -111,9 +111,9 @@ static void start_object_request(struct walker *walker,
        struct walker_data *data = walker->data;
 
        snprintf(prevfile, sizeof(prevfile), "%s.prev", obj_req->filename);
-       unlink(prevfile);
+       unlink_or_warn(prevfile);
        rename(obj_req->tmpfile, prevfile);
-       unlink(obj_req->tmpfile);
+       unlink_or_warn(obj_req->tmpfile);
 
        if (obj_req->local != -1)
                error("fd leakage in start: %d", obj_req->local);
@@ -177,7 +177,7 @@ static void start_object_request(struct walker *walker,
                } while (prev_read > 0);
                close(prevlocal);
        }
-       unlink(prevfile);
+       unlink_or_warn(prevfile);
 
        /* Reset inflate/SHA1 if there was an error reading the previous temp
           file; also rewind to the beginning of the local file. */
@@ -238,18 +238,18 @@ static void finish_object_request(struct object_request *obj_req)
        } else if (obj_req->curl_result != CURLE_OK) {
                if (stat(obj_req->tmpfile, &st) == 0)
                        if (st.st_size == 0)
-                               unlink(obj_req->tmpfile);
+                               unlink_or_warn(obj_req->tmpfile);
                return;
        }
 
        git_inflate_end(&obj_req->stream);
        git_SHA1_Final(obj_req->real_sha1, &obj_req->c);
        if (obj_req->zret != Z_STREAM_END) {
-               unlink(obj_req->tmpfile);
+               unlink_or_warn(obj_req->tmpfile);
                return;
        }
        if (hashcmp(obj_req->sha1, obj_req->real_sha1)) {
-               unlink(obj_req->tmpfile);
+               unlink_or_warn(obj_req->tmpfile);
                return;
        }
        obj_req->rename =
@@ -809,7 +809,7 @@ static void abort_object_request(struct object_request *obj_req)
                close(obj_req->local);
                obj_req->local = -1;
        }
-       unlink(obj_req->tmpfile);
+       unlink_or_warn(obj_req->tmpfile);
        if (obj_req->slot) {
                release_active_slot(obj_req->slot);
                obj_req->slot = NULL;

diff --git a/ll-merge.c b/ll-merge.c
index fa2ca5250c75a0d335570a3a3c71b20ebd0b42fa..81c02ad0531e98379d8cd11dc7c2d70214d67fb4 100644 (file)
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -219,7 +219,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
        close(fd);
  bad:
        for (i = 0; i < 3; i++)
-               unlink(temp[i]);
+               unlink_or_warn(temp[i]);
        strbuf_release(&cmd);
        return status;
 }

diff --git a/lockfile.c b/lockfile.c
index 3dbb2d1ff9407a7417be294adedc5312e7421a96..984eb320fc7b3c97537556f22bef8077d52b389d 100644 (file)
--- a/lockfile.c
+++ b/lockfile.c
@@ -16,7 +16,7 @@ static void remove_lock_file(void)
                    lock_file_list->filename[0]) {
                        if (lock_file_list->fd >= 0)
                                close(lock_file_list->fd);
-                       unlink(lock_file_list->filename);
+                       unlink_or_warn(lock_file_list->filename);
                }
                lock_file_list = lock_file_list->next;
        }
@@ -259,7 +259,7 @@ void rollback_lock_file(struct lock_file *lk)
        if (lk->filename[0]) {
                if (lk->fd >= 0)
                        close(lk->fd);
-               unlink(lk->filename);
+               unlink_or_warn(lk->filename);
        }
        lk->filename[0] = 0;
 }

diff --git a/pack-refs.c b/pack-refs.c
index 2c76fb181f64e10c65517224b0c09cb648db81ac..301fc60eae1ad53721851a272a0fbd0192881801 100644 (file)
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -66,7 +66,7 @@ static void prune_ref(struct ref_to_prune *r)
        struct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);
 
        if (lock) {
-               unlink(git_path("%s", r->name));
+               unlink_or_warn(git_path("%s", r->name));
                unlock_ref(lock);
        }
 }

diff --git a/refs.c b/refs.c
index e65a3b4c4ef57863a1055108d2598777cabc2c8d..2b1f0f0e6ed7978107e5b488f8d12f6276e482a4 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -1002,12 +1002,10 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
                } else {
                        path = git_path("%s", refname);
                }
-               err = unlink(path);
-               if (err && errno != ENOENT) {
+               err = unlink_or_warn(path);
+               if (err && errno != ENOENT)
                        ret = 1;
-                       error("unlink(%s) failed: %s",
-                             path, strerror(errno));
-               }
+
                if (!(delopt & REF_NODEREF))
                        lock->lk->filename[i] = '.';
        }
@@ -1017,10 +1015,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
         */
        ret |= repack_without_ref(refname);
 
-       err = unlink(git_path("logs/%s", lock->ref_name));
-       if (err && errno != ENOENT)
-               warning("unlink(%s) failed: %s",
-                       git_path("logs/%s", lock->ref_name), strerror(errno));
+       unlink_or_warn(git_path("logs/%s", lock->ref_name));
        invalidate_cached_refs();
        unlock_ref(lock);
        return ret;
@@ -1381,7 +1376,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
        if (adjust_shared_perm(git_HEAD)) {
                error("Unable to fix permissions on %s", lockpath);
        error_unlink_return:
-               unlink(lockpath);
+               unlink_or_warn(lockpath);
        error_free_return:
                free(git_HEAD);
                return -1;

diff --git a/rerere.c b/rerere.c
index 713c6e16ac536eaa667cc9f22768670be129483c..87360dc23e54aabf47b3bda160e58058507d4a2f 100644 (file)
--- a/rerere.c
+++ b/rerere.c
@@ -173,7 +173,7 @@ static int handle_file(const char *path,
                git_SHA1_Final(sha1, &ctx);
        if (hunk != RR_CONTEXT) {
                if (output)
-                       unlink(output);
+                       unlink_or_warn(output);
                return error("Could not parse conflict hunks in %s", path);
        }
        if (wrerror)

diff --git a/server-info.c b/server-info.c
index 66b0d9d878a011393582b837301eb1fd5caf2e40..d096dc7718b21eb7f5e88b7276983ca6cccc723e 100644 (file)
--- a/server-info.c
+++ b/server-info.c
@@ -246,7 +246,7 @@ int update_server_info(int force)
        errs = errs | update_info_packs(force);
 
        /* remove leftover rev-cache file if there is any */
-       unlink(git_path("info/rev-cache"));
+       unlink_or_warn(git_path("info/rev-cache"));
 
        return errs;
 }

diff --git a/sha1_file.c b/sha1_file.c
index f708cf4f674fdf393044d6c36c7d97f1fa7dcd0c..dd474116a8856b429c9416de5c3a149e11a9f70b 100644 (file)
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2247,7 +2247,7 @@ int move_temp_to_file(const char *tmpfile, const char *filename)
                        goto out;
                ret = errno;
        }
-       unlink(tmpfile);
+       unlink_or_warn(tmpfile);
        if (ret) {
                if (ret != EEXIST) {
                        return error("unable to write sha1 filename %s: %s\n", filename, strerror(ret));

diff --git a/transport.c b/transport.c
index 3dfb03c06ed82102f60045d6889db97cf036651e..efecb65258ba95ee743f0e3ca178e74e2756e7d1 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -1069,7 +1069,7 @@ int transport_fetch_refs(struct transport *transport, const struct ref *refs)
 void transport_unlock_pack(struct transport *transport)
 {
        if (transport->pack_lockfile) {
-               unlink(transport->pack_lockfile);
+               unlink_or_warn(transport->pack_lockfile);
                free(transport->pack_lockfile);
                transport->pack_lockfile = NULL;
        }

diff --git a/unpack-trees.c b/unpack-trees.c
index e4eb8fa3afc7ad895ba925701ee3c6881b29bd91..aaacaf1015ccf1f353151982a3018ad349663d76 100644 (file)
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -61,7 +61,7 @@ static void unlink_entry(struct cache_entry *ce)
 {
        if (has_symlink_or_noent_leading_path(ce->name, ce_namelen(ce)))
                return;
-       if (unlink(ce->name))
+       if (unlink_or_warn(ce->name))
                return;
        schedule_dir_for_removal(ce->name, ce_namelen(ce));
 }