prefer mkpathdup to mkpath in assignments

As with the previous commit to git_path, assigning the
result of mkpath is suspicious, since it is not clear
whether we will still depend on the value after it may have
been overwritten by subsequent calls. This patch converts
low-hanging fruit to use mkpathdup instead of mkpath (with
the downside that we must remember to free the result).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/repack.c b/builtin/repack.c
index af7340c7bafbfbbf991f782139775e18b6ac570a..70b9b1eaf17f5447021f7174f31e5c19d9754486 100644 (file)
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -285,8 +285,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
        failed = 0;
        for_each_string_list_item(item, &names) {
                for (ext = 0; ext < ARRAY_SIZE(exts); ext++) {
        failed = 0;
        for_each_string_list_item(item, &names) {
                for (ext = 0; ext < ARRAY_SIZE(exts); ext++) {

-                       const char *fname_old;
-                       char *fname;
+                       char *fname, *fname_old;

                        fname = mkpathdup("%s/pack-%s%s", packdir,
                                                item->string, exts[ext].name);
                        if (!file_exists(fname)) {
                        fname = mkpathdup("%s/pack-%s%s", packdir,
                                                item->string, exts[ext].name);
                        if (!file_exists(fname)) {


@@ -294,7 +293,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
                                continue;
                        }
 
                                continue;
                        }
 

-                       fname_old = mkpath("%s/old-%s%s", packdir,
+                       fname_old = mkpathdup("%s/old-%s%s", packdir,

                                                item->string, exts[ext].name);
                        if (file_exists(fname_old))
                                if (unlink(fname_old))
                                                item->string, exts[ext].name);
                        if (file_exists(fname_old))
                                if (unlink(fname_old))


@@ -302,10 +301,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 
                        if (!failed && rename(fname, fname_old)) {
                                free(fname);
 
                        if (!failed && rename(fname, fname_old)) {
                                free(fname);

+                               free(fname_old);

                                failed = 1;
                                break;
                        } else {
                                string_list_append(&rollback, fname);
                                failed = 1;
                                break;
                        } else {
                                string_list_append(&rollback, fname);

+                               free(fname_old);

                        }
                }
                if (failed)
                        }
                }
                if (failed)


@@ -314,13 +315,13 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
        if (failed) {
                struct string_list rollback_failure = STRING_LIST_INIT_DUP;
                for_each_string_list_item(item, &rollback) {
        if (failed) {
                struct string_list rollback_failure = STRING_LIST_INIT_DUP;
                for_each_string_list_item(item, &rollback) {

-                       const char *fname_old;
-                       char *fname;
+                       char *fname, *fname_old;

                        fname = mkpathdup("%s/%s", packdir, item->string);
                        fname = mkpathdup("%s/%s", packdir, item->string);

-                       fname_old = mkpath("%s/old-%s", packdir, item->string);
+                       fname_old = mkpathdup("%s/old-%s", packdir, item->string);

                        if (rename(fname_old, fname))
                                string_list_append(&rollback_failure, fname);
                        free(fname);
                        if (rename(fname_old, fname))
                                string_list_append(&rollback_failure, fname);
                        free(fname);

+                       free(fname_old);

                }
 
                if (rollback_failure.nr) {
                }
 
                if (rollback_failure.nr) {


@@ -368,13 +369,14 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
        /* Remove the "old-" files */
        for_each_string_list_item(item, &names) {
                for (ext = 0; ext < ARRAY_SIZE(exts); ext++) {
        /* Remove the "old-" files */
        for_each_string_list_item(item, &names) {
                for (ext = 0; ext < ARRAY_SIZE(exts); ext++) {

-                       const char *fname;
-                       fname = mkpath("%s/old-%s%s",
-                                       packdir,
-                                       item->string,
-                                       exts[ext].name);
+                       char *fname;
+                       fname = mkpathdup("%s/old-%s%s",
+                                         packdir,
+                                         item->string,
+                                         exts[ext].name);

                        if (remove_path(fname))
                                warning(_("removing '%s' failed"), fname);
                        if (remove_path(fname))
                                warning(_("removing '%s' failed"), fname);

+                       free(fname);

                }
        }
 
                }
        }
 

diff --git a/refs.c b/refs.c
index 93b250e75413cb03ed9737a8bc2fe8990afaa544..e70941a090b96fd04e2f9f68d422fd0b7e0cd7d9 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -3380,7 +3380,7 @@ static int commit_ref_update(struct ref_lock *lock,
 int create_symref(const char *ref_target, const char *refs_heads_master,
                  const char *logmsg)
 {
 int create_symref(const char *ref_target, const char *refs_heads_master,
                  const char *logmsg)
 {

-       const char *lockpath;
+       char *lockpath = NULL;

        char ref[1000];
        int fd, len, written;
        char *git_HEAD = git_pathdup("%s", ref_target);
        char ref[1000];
        int fd, len, written;
        char *git_HEAD = git_pathdup("%s", ref_target);


@@ -3407,7 +3407,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
                error("refname too long: %s", refs_heads_master);
                goto error_free_return;
        }
                error("refname too long: %s", refs_heads_master);
                goto error_free_return;
        }

-       lockpath = mkpath("%s.lock", git_HEAD);
+       lockpath = mkpathdup("%s.lock", git_HEAD);

        fd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666);
        if (fd < 0) {
                error("Unable to open %s for writing", lockpath);
        fd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666);
        if (fd < 0) {
                error("Unable to open %s for writing", lockpath);


@@ -3427,9 +3427,11 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
        error_unlink_return:
                unlink_or_warn(lockpath);
        error_free_return:
        error_unlink_return:
                unlink_or_warn(lockpath);
        error_free_return:

+               free(lockpath);

                free(git_HEAD);
                return -1;
        }
                free(git_HEAD);
                return -1;
        }

+       free(lockpath);

 
 #ifndef NO_SYMLINK_HEAD
        done:
 
 #ifndef NO_SYMLINK_HEAD
        done: