Skip to content

Commit

Permalink
prefer mkpathdup to mkpath in assignments
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Aug 10, 2015
1 parent fcd12db commit e3cf230
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 13 deletions.
24 changes: 13 additions & 11 deletions builtin/repack.c
Original file line number Diff line number Diff line change
Expand Up @@ -285,27 +285,28 @@ 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++) {
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)) {
free(fname);
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))
failed = 1;

if (!failed && rename(fname, fname_old)) {
free(fname);
free(fname_old);
failed = 1;
break;
} else {
string_list_append(&rollback, fname);
free(fname_old);
}
}
if (failed)
Expand All @@ -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) {
const char *fname_old;
char *fname;
char *fname, *fname_old;
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);
free(fname_old);
}

if (rollback_failure.nr) {
Expand Down Expand Up @@ -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++) {
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);
free(fname);
}
}

Expand Down
6 changes: 4 additions & 2 deletions refs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
const char *lockpath;
char *lockpath = NULL;
char ref[1000];
int fd, len, written;
char *git_HEAD = git_pathdup("%s", ref_target);
Expand All @@ -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;
}
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);
Expand All @@ -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:
free(lockpath);
free(git_HEAD);
return -1;
}
free(lockpath);

#ifndef NO_SYMLINK_HEAD
done:
Expand Down

0 comments on commit e3cf230

Please sign in to comment.