git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] builtin-tag.c: Fix two memory leaks and minor notation changes.

From
CRCarlos Rica <jasampler@gmail.com>
Date
Jul 21, 2007, 12:13 UTC
Message-ID
<46A1F858.7080405@gmail.com>

A repeated call to read_sha1_file was not freing memory when the buffer was allocated but returned size was zero.

Also, now the program does not allow many -F or -m options, which was a bug too because it was not freing the memory allocated for any previous -F or -m options.

Tests are provided for ensuring that only one option -F or -m is given. Also, another test is shipped here, to check that "git tag" fails when a non-existing file is passed to the -F option, something that git-tag.sh allowed creating the tag with an empty message.

Signed-off-by: Carlos Rica <jasampler@gmail.com>
---
  Those were commented in the responses to my previous patch:
  http://article.gmane.org/gmane.comp.version-control.git/53138
  I committed them on top of that my other patch already in "next".
  Unfortunately, I was not able to commit before the patches
  from Johannes supporting the EDITOR environment variable
  in builtin-tag.c and fixing its bug with a non-initialized buffer.
 builtin-tag.c  |   38 ++++++++++++++++++++++----------------
 t/t7004-tag.sh |   35 +++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 16 deletions(-)
diff --git a/builtin-tag.c b/builtin-tag.c
index 507f510..9231d72 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -85,14 +85,19 @@ static int show_reference(const char *refname, const unsigned char *sha1,
 		printf("%-15s ", refname);

 		sp = buf = read_sha1_file(sha1, &type, &size);
-		if (!buf || !size)
+		if (!buf)
 			return 0;
+		if (!size) {
+			free(buf);
+			return 0;
+		}
 		/* skip header */
 		while (sp + 1 < buf + size &&
 				!(sp[0] == '\n' && sp[1] == '\n'))
 			sp++;
 		/* only take up to "lines" lines, and strip the signature */
-		for (i = 0, sp += 2; i < filter->lines && sp < buf + size &&
+		for (i = 0, sp += 2;
+				i < filter->lines && sp < buf + size &&
 				prefixcmp(sp, PGP_SIGNATURE "\n");
 				i++) {
 			if (i)
@@ -133,10 +138,10 @@ static int list_tags(const char *pattern, int lines)
 	return 0;
 }

-typedef int (*func_tag)(const char *name, const char *ref,
+typedef int (*each_tag_name_fn)(const char *name, const char *ref,
 				const unsigned char *sha1);

-static int do_tag_names(const char **argv, func_tag fn)
+static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
 {
 	const char **p;
 	char ref[PATH_MAX];
@@ -191,7 +196,7 @@ static ssize_t do_sign(char *buffer, size_t size, size_t max)

 	if (!*signingkey) {
 		if (strlcpy(signingkey, git_committer_info(1),
-				sizeof(signingkey)) >= sizeof(signingkey))
+				sizeof(signingkey)) > sizeof(signingkey) - 1)
 			return error("committer info too long.");
 		bracket = strchr(signingkey, '>');
 		if (bracket)
@@ -254,7 +259,7 @@ static void create_tag(const unsigned char *object, const char *tag,
 	unsigned long size;

 	type = sha1_object_info(object, NULL);
-	if (type <= 0)
+	if (type <= OBJ_NONE)
 	    die("bad object type.");

 	header_len = snprintf(header_buf, sizeof(header_buf),
@@ -267,7 +272,7 @@ static void create_tag(const unsigned char *object, const char *tag,
 			  tag,
 			  git_committer_info(1));

-	if (header_len >= sizeof(header_buf))
+	if (header_len > sizeof(header_buf) - 1)
 		die("tag header too big.");

 	if (!message) {
@@ -362,6 +367,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 			i++;
 			if (i == argc)
 				die("option -m needs an argument.");
+			if (message)
+				die("only one -F or -m option is allowed.");
 			message = xstrdup(argv[i]);
 			continue;
 		}
@@ -373,6 +380,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 			i++;
 			if (i == argc)
 				die("option -F needs an argument.");
+			if (message)
+				die("only one -F or -m option is allowed.");

 			if (!strcmp(argv[i], "-"))
 				fd = 0;
@@ -401,15 +410,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 				die("argument to option -u too long");
 			continue;
 		}
-		if (!strcmp(arg, "-l")) {
+		if (!strcmp(arg, "-l"))
 			return list_tags(argv[i + 1], lines);
-		}
-		if (!strcmp(arg, "-d")) {
-			return do_tag_names(argv + i + 1, delete_tag);
-		}
-		if (!strcmp(arg, "-v")) {
-			return do_tag_names(argv + i + 1, verify_tag);
-		}
+		if (!strcmp(arg, "-d"))
+			return for_each_tag_name(argv + i + 1, delete_tag);
+		if (!strcmp(arg, "-v"))
+			return for_each_tag_name(argv + i + 1, verify_tag);
 		usage(builtin_tag_usage);
 	}

@@ -427,7 +433,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (get_sha1(object_ref, object))
 		die("Failed to resolve '%s' as a valid ref.", object_ref);

-	if (snprintf(ref, sizeof(ref), "refs/tags/%s", tag) >= sizeof(ref))
+	if (snprintf(ref, sizeof(ref), "refs/tags/%s", tag) > sizeof(ref) - 1)
 		die("tag name too long: %.*s...", 50, tag);
 	if (check_ref_format(ref))
 		die("'%s' is not a valid tag name.", tag);
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index a0be164..c4fa446 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -332,6 +332,33 @@ test_expect_success 'creating an annotated tag with -F - should succeed' '
 	git diff expect actual
 '

+test_expect_success \
+	'trying to create a tag with a non-existing -F file should fail' '
+	! test -f nonexistingfile &&
+	! tag_exists notag &&
+	! git-tag -F nonexistingfile notag &&
+	! tag_exists notag
+'
+
+test_expect_success \
+	'trying to create tags giving many -m or -F options should fail' '
+	echo "message file 1" >msgfile1 &&
+	echo "message file 2" >msgfile2 &&
+	! tag_exists msgtag &&
+	! git-tag -m "message 1" -m "message 2" msgtag &&
+	! tag_exists msgtag &&
+	! git-tag -F msgfile1 -F msgfile2 msgtag &&
+	! tag_exists msgtag &&
+	! git-tag -m "message 1" -F msgfile1 msgtag &&
+	! tag_exists msgtag &&
+	! git-tag -F msgfile1 -m "message 1" msgtag &&
+	! tag_exists msgtag &&
+	! git-tag -F msgfile1 -m "message 1" -F msgfile2 msgtag &&
+	! tag_exists msgtag &&
+	! git-tag -m "message 1" -F msgfile1 -m "message 2" msgtag &&
+	! tag_exists msgtag
+'
+
 # blank and empty messages:

 get_tag_header empty-annotated-tag $commit commit $time >expect
@@ -648,6 +675,14 @@ test_expect_success 'creating a signed tag with -F - should succeed' '
 	git diff expect actual
 '

+test_expect_success \
+	'trying to create a signed tag with non-existing -F file should fail' '
+	! test -f nonexistingfile &&
+	! tag_exists nosigtag &&
+	! git-tag -s -F nonexistingfile nosigtag &&
+	! tag_exists nosigtag
+'
+
 test_expect_success 'verifying a signed tag should succeed' \
 	'git-tag -v signed-tag'

-- 
1.5.0
Next: Carlos Rica
Message 1 of 3 in “builtin-tag.c: Fix two memory leaks and minor notation changes.”
  1. builtin-tag.c: Fix two memory leaks and minor notation changes.Carlos Rica, Jul 21, 2007
  2. Carlos RicaJul 21, 2007
  3. Johannes SchindelinJul 21, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.