git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH 07/17] prefer mkpathdup to mkpath in assignments
Date: Mon, 10 Aug 2015 05:35:38 -0400	[thread overview]
Message-ID: <20150810093538.GG30981@sigill.intra.peff.net> (raw)
In-Reply-To: <20150810092731.GA9027@sigill.intra.peff.net>

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>
---
 builtin/repack.c | 24 +++++++++++++-----------
 refs.c           |  6 ++++--
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index af7340c..70b9b1e 100644
--- 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++) {
-			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)) {
@@ -294,7 +293,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 				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))
@@ -302,10 +301,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 
 			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)
@@ -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) {
@@ -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);
 		}
 	}
 
diff --git a/refs.c b/refs.c
index 93b250e..e70941a 100644
--- 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)
 {
-	const char *lockpath;
+	char *lockpath = NULL;
 	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;
 	}
-	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);
@@ -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:
-- 
2.5.0.414.g670f2a4

  parent reply	other threads:[~2015-08-10  9:35 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-10  9:27 [PATCH 0/17] removing questionable uses of git_path Jeff King
2015-08-10  9:32 ` [PATCH 01/17] cache.h: clarify documentation for git_path, et al Jeff King
2015-08-10  9:32 ` [PATCH 02/17] cache.h: complete set of git_path_submodule helpers Jeff King
2015-08-10  9:32 ` [PATCH 03/17] t5700: modernize style Jeff King
2015-08-10  9:34 ` [PATCH 04/17] add_to_alternates_file: don't add duplicate entries Jeff King
2015-08-11  4:00   ` Michael Haggerty
2015-08-11  9:54     ` Jeff King
2015-08-10  9:35 ` [PATCH 05/17] remove hold_lock_file_for_append Jeff King
2015-08-10 22:36   ` Junio C Hamano
2015-08-11  9:38     ` Jeff King
2015-08-10  9:35 ` [PATCH 06/17] prefer git_pathdup to git_path in some possibly-dangerous cases Jeff King
2015-08-10  9:35 ` Jeff King [this message]
2015-08-10  9:35 ` [PATCH 08/17] remote.c: drop extraneous local variable from migrate_file Jeff King
2015-08-10  9:36 ` [PATCH 09/17] refs.c: remove extra git_path calls from read_loose_refs Jeff King
2015-08-10  9:36 ` [PATCH 10/17] path.c: drop git_path_submodule Jeff King
2015-08-10 22:50   ` Junio C Hamano
2015-08-10 22:57     ` Junio C Hamano
2015-08-10 23:52       ` Junio C Hamano
2015-08-11  9:53       ` Jeff King
2015-08-10  9:36 ` [PATCH 11/17] refs.c: simplify strbufs in reflog setup and writing Jeff King
2015-08-10 10:34   ` Michael Haggerty
2015-08-10 12:26     ` Jeff King
2015-08-10  9:36 ` [PATCH 12/17] refs.c: avoid repeated git_path calls in rename_tmp_log Jeff King
2015-08-10  9:37 ` [PATCH 13/17] refs.c: avoid git_path assignment in lock_ref_sha1_basic Jeff King
2015-08-10  9:37 ` [PATCH 14/17] refs.c: remove_empty_directories can take a strbuf Jeff King
2015-08-10  9:37 ` [PATCH 15/17] find_hook: keep our own static buffer Jeff King
2015-08-10  9:37 ` [PATCH 16/17] get_repo_path: refactor path-allocation Jeff King
2015-08-10  9:38 ` [PATCH 17/17] memoize common git-path "constant" files Jeff King
2015-08-10 12:05   ` Michael Haggerty
2015-08-10 12:30     ` Jeff King
2015-08-10 12:06 ` [PATCH 0/17] removing questionable uses of git_path Michael Haggerty
2015-08-10 17:31 ` Junio C Hamano
2015-08-10 17:47   ` Jeff King
2015-08-15  9:05 ` Duy Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150810093538.GG30981@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).