git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "René Scharfe" <l.s.r@web.de>, git@vger.kernel.org
Subject: [PATCH v2 05/10] use xstrfmt to replace xmalloc + strcpy/strcat
Date: Thu, 19 Jun 2014 17:26:56 -0400	[thread overview]
Message-ID: <20140619212656.GE28474@sigill.intra.peff.net> (raw)
In-Reply-To: <20140619211659.GA32412@sigill.intra.peff.net>

It's easy to get manual allocation calculations wrong, and
the use of strcpy/strcat raise red flags for people looking
for buffer overflows (though in this case each site was
fine).

It's also shorter to use xstrfmt, and the printf-format
tends to be easier for a reader to see what the final string
will look like.

Signed-off-by: Jeff King <peff@peff.net>
---
By the way, I think that the tip_name allocation in name_rev leaks
badly, but it's a little tricky to fix (we sometimes hand off ownership
of the variable, and sometimes not). However, this patch does not make
it any worse, and nobody is complaining, so I left it for now.

 builtin/apply.c    | 4 +---
 builtin/fetch.c    | 9 ++-------
 builtin/name-rev.c | 5 +----
 sha1_name.c        | 5 +----
 shell.c            | 6 +-----
 5 files changed, 6 insertions(+), 23 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 9c5724e..b796910 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1281,9 +1281,7 @@ static int parse_git_header(const char *line, int len, unsigned int size, struct
 	 */
 	patch->def_name = git_header_name(line, len);
 	if (patch->def_name && root) {
-		char *s = xmalloc(root_len + strlen(patch->def_name) + 1);
-		strcpy(s, root);
-		strcpy(s + root_len, patch->def_name);
+		char *s = xstrfmt("%s%s", root, patch->def_name);
 		free(patch->def_name);
 		patch->def_name = s;
 	}
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 55f457c..40d989f 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1053,16 +1053,11 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 		refs = xcalloc(argc + 1, sizeof(const char *));
 		for (i = 0; i < argc; i++) {
 			if (!strcmp(argv[i], "tag")) {
-				char *ref;
 				i++;
 				if (i >= argc)
 					die(_("You need to specify a tag name."));
-				ref = xmalloc(strlen(argv[i]) * 2 + 22);
-				strcpy(ref, "refs/tags/");
-				strcat(ref, argv[i]);
-				strcat(ref, ":refs/tags/");
-				strcat(ref, argv[i]);
-				refs[j++] = ref;
+				refs[j++] = xstrfmt("refs/tags/%s:refs/tags/%s",
+						    argv[i], argv[i]);
 			} else
 				refs[j++] = argv[i];
 		}
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index c824d4e..3c8f319 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -33,10 +33,7 @@ static void name_rev(struct commit *commit,
 		return;
 
 	if (deref) {
-		char *new_name = xmalloc(strlen(tip_name)+3);
-		strcpy(new_name, tip_name);
-		strcat(new_name, "^0");
-		tip_name = new_name;
+		tip_name = xstrfmt("%s^0", tip_name);
 
 		if (generation)
 			die("generation: %d, but deref?", generation);
diff --git a/sha1_name.c b/sha1_name.c
index 2b6322f..5e95690 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1252,10 +1252,7 @@ static void diagnose_invalid_sha1_path(const char *prefix,
 		die("Path '%s' exists on disk, but not in '%.*s'.",
 		    filename, object_name_len, object_name);
 	if (errno == ENOENT || errno == ENOTDIR) {
-		char *fullname = xmalloc(strlen(filename)
-					     + strlen(prefix) + 1);
-		strcpy(fullname, prefix);
-		strcat(fullname, filename);
+		char *fullname = xstrfmt("%s%s", prefix, filename);
 
 		if (!get_tree_entry(tree_sha1, fullname,
 				    sha1, &mode)) {
diff --git a/shell.c b/shell.c
index 5c0d47a..ace62e4 100644
--- a/shell.c
+++ b/shell.c
@@ -46,11 +46,7 @@ static int is_valid_cmd_name(const char *cmd)
 
 static char *make_cmd(const char *prog)
 {
-	char *prefix = xmalloc((strlen(prog) + strlen(COMMAND_DIR) + 2));
-	strcpy(prefix, COMMAND_DIR);
-	strcat(prefix, "/");
-	strcat(prefix, prog);
-	return prefix;
+	return xstrfmt("%s/%s", COMMAND_DIR, prog);
 }
 
 static void cd_to_homedir(void)
-- 
2.0.0.566.gfe3e6b2

  parent reply	other threads:[~2014-06-19 21:27 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-18 20:00 [PATCH 0/2] dropping manual malloc calculations Jeff King
2014-06-18 20:01 ` [PATCH 1/2] strbuf: add xstrdup_fmt helper Jeff King
2014-06-18 22:32   ` Junio C Hamano
2014-06-19  9:05     ` Jeff King
2014-06-19 16:49       ` Junio C Hamano
2014-06-19 21:16         ` [PATCH v2] dropping manual malloc calculations Jeff King
2014-06-19 21:18           ` [PATCH v2 01/10] strbuf: add xstrfmt helper Jeff King
2014-06-19 21:19           ` [PATCH v2 02/10] use xstrfmt in favor of manual size calculations Jeff King
2014-06-19 21:19           ` [PATCH v2 03/10] use xstrdup instead of xmalloc + strcpy Jeff King
2014-06-19 21:24           ` [PATCH v2 04/10] use xstrfmt to replace xmalloc + sprintf Jeff King
2014-06-19 21:26           ` Jeff King [this message]
2014-06-19 21:28           ` [PATCH v2 06/10] setup_git_env: use git_pathdup instead of " Jeff King
2014-06-23 10:21             ` Eric Sunshine
2014-06-23 22:43               ` Junio C Hamano
2014-06-24 13:02               ` Duy Nguyen
2014-06-24 13:30             ` Duy Nguyen
2014-06-24 20:58               ` Jeff King
2014-06-25 12:37                 ` Duy Nguyen
2014-06-25 17:20                 ` Junio C Hamano
2014-06-25 17:22                   ` Jeff King
2014-06-25 19:54                     ` Junio C Hamano
2014-06-19 21:28           ` [PATCH v2 07/10] sequencer: use argv_array_pushf Jeff King
2014-06-19 21:29           ` [PATCH v2 08/10] merge: use argv_array when spawning merge strategy Jeff King
2014-06-19 21:29           ` [PATCH v2 09/10] walker_fetch: fix minor memory leak Jeff King
2014-06-19 21:30           ` [PATCH v2 10/10] unique_path: fix unlikely heap overflow Jeff King
2014-06-19 16:52       ` [PATCH 1/2] strbuf: add xstrdup_fmt helper René Scharfe
2014-06-18 20:02 ` [PATCH 2/2] use xstrdup_fmt in favor of manual size calculations Jeff King

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

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

  git send-email \
    --in-reply-to=20140619212656.GE28474@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    /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.
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 NNTP newsgroup(s).