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>

Jeff King committed Aug 10, 2015 at 05:35 UTC e3cf230324f740653d4fb4a3087c2daf9da62029
2 files changed +17 -13
builtin/repack.c
+13 -11
@@ -285,8 +285,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
285 failed = 0;
286 for_each_string_list_item(item, &names) {
287 for (ext = 0; ext < ARRAY_SIZE(exts); ext++) {
288 - const char *fname_old;
289 - char *fname;
288 + char *fname, *fname_old;
289 fname = mkpathdup("%s/pack-%s%s", packdir,
290 item->string, exts[ext].name);
291 if (!file_exists(fname)) {
@@ -294,7 +293,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
293 continue;
294 }
295
297 - fname_old = mkpath("%s/old-%s%s", packdir,
296 + fname_old = mkpathdup("%s/old-%s%s", packdir,
297 item->string, exts[ext].name);
298 if (file_exists(fname_old))
299 if (unlink(fname_old))
@@ -302,10 +301,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
301
302 if (!failed && rename(fname, fname_old)) {
303 free(fname);
304 + free(fname_old);
305 failed = 1;
306 break;
307 } else {
308 string_list_append(&rollback, fname);
309 + free(fname_old);
310 }
311 }
312 if (failed)
@@ -314,13 +315,13 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
315 if (failed) {
316 struct string_list rollback_failure = STRING_LIST_INIT_DUP;
317 for_each_string_list_item(item, &rollback) {
317 - const char *fname_old;
318 - char *fname;
318 + char *fname, *fname_old;
319 fname = mkpathdup("%s/%s", packdir, item->string);
320 - fname_old = mkpath("%s/old-%s", packdir, item->string);
320 + fname_old = mkpathdup("%s/old-%s", packdir, item->string);
321 if (rename(fname_old, fname))
322 string_list_append(&rollback_failure, fname);
323 free(fname);
324 + free(fname_old);
325 }
326
327 if (rollback_failure.nr) {
@@ -368,13 +369,14 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
369 /* Remove the "old-" files */
370 for_each_string_list_item(item, &names) {
371 for (ext = 0; ext < ARRAY_SIZE(exts); ext++) {
371 - const char *fname;
372 - fname = mkpath("%s/old-%s%s",
373 - packdir,
374 - item->string,
375 - exts[ext].name);
372 + char *fname;
373 + fname = mkpathdup("%s/old-%s%s",
374 + packdir,
375 + item->string,
376 + exts[ext].name);
377 if (remove_path(fname))
378 warning(_("removing '%s' failed"), fname);
379 + free(fname);
380 }
381 }
382
refs.c
+4 -2
@@ -3380,7 +3380,7 @@ static int commit_ref_update(struct ref_lock *lock,
3380 int create_symref(const char *ref_target, const char *refs_heads_master,
3381 const char *logmsg)
3382 {
3383 - const char *lockpath;
3383 + char *lockpath = NULL;
3384 char ref[1000];
3385 int fd, len, written;
3386 char *git_HEAD = git_pathdup("%s", ref_target);
@@ -3407,7 +3407,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
3407 error("refname too long: %s", refs_heads_master);
3408 goto error_free_return;
3409 }
3410 - lockpath = mkpath("%s.lock", git_HEAD);
3410 + lockpath = mkpathdup("%s.lock", git_HEAD);
3411 fd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666);
3412 if (fd < 0) {
3413 error("Unable to open %s for writing", lockpath);
@@ -3427,9 +3427,11 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
3427 error_unlink_return:
3428 unlink_or_warn(lockpath);
3429 error_free_return:
3430 + free(lockpath);
3431 free(git_HEAD);
3432 return -1;
3433 }
3434 + free(lockpath);
3435
3436 #ifndef NO_SYMLINK_HEAD
3437 done: