{"thread":{"id":"9126","subject":"[PATCH] builtin-tag.c: Fix two memory leaks and minor notation changes.","startedAt":"2007-07-21T12:13:12Z","lastAt":"2007-07-21T13:08:26Z","messageCount":3,"participants":["Carlos Rica","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"48030","messageId":"46A1F858.7080405@gmail.com","threadId":"9126","inReplyTo":null,"subject":"[PATCH] builtin-tag.c: Fix two memory leaks and minor notation changes.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2007-07-21T12:13:12Z","receivedAt":"2007-07-21T12:13:12Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"A repeated call to read_sha1_file was not freing memory\nwhen the buffer was allocated but returned size was zero.\n\nAlso, now the program does not allow many -F or -m options,\nwhich was a bug too because it was not freing the memory\nallocated for any previous -F or -m options.\n\nTests are provided for ensuring that only one option\n-F or -m is given. Also, another test is shipped here,\nto check that \"git tag\" fails when a non-existing file\nis passed to the -F option, something that git-tag.sh\nallowed creating the tag with an empty message.\n\nSigned-off-by: Carlos Rica <jasampler@gmail.com>\n---\n\n  Those were commented in the responses to my previous patch:\n  http://article.gmane.org/gmane.comp.version-control.git/53138\n\n  I committed them on top of that my other patch already in \"next\".\n  Unfortunately, I was not able to commit before the patches\n  from Johannes supporting the EDITOR environment variable\n  in builtin-tag.c and fixing its bug with a non-initialized buffer.\n\n builtin-tag.c  |   38 ++++++++++++++++++++++----------------\n t/t7004-tag.sh |   35 +++++++++++++++++++++++++++++++++++\n 2 files changed, 57 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 507f510..9231d72 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -85,14 +85,19 @@ static int show_reference(const char *refname, const unsigned char *sha1,\n \t\tprintf(\"%-15s \", refname);\n\n \t\tsp = buf = read_sha1_file(sha1, &type, &size);\n-\t\tif (!buf || !size)\n+\t\tif (!buf)\n \t\t\treturn 0;\n+\t\tif (!size) {\n+\t\t\tfree(buf);\n+\t\t\treturn 0;\n+\t\t}\n \t\t/* skip header */\n \t\twhile (sp + 1 < buf + size &&\n \t\t\t\t!(sp[0] == '\\n' && sp[1] == '\\n'))\n \t\t\tsp++;\n \t\t/* only take up to \"lines\" lines, and strip the signature */\n-\t\tfor (i = 0, sp += 2; i < filter->lines && sp < buf + size &&\n+\t\tfor (i = 0, sp += 2;\n+\t\t\t\ti < filter->lines && sp < buf + size &&\n \t\t\t\tprefixcmp(sp, PGP_SIGNATURE \"\\n\");\n \t\t\t\ti++) {\n \t\t\tif (i)\n@@ -133,10 +138,10 @@ static int list_tags(const char *pattern, int lines)\n \treturn 0;\n }\n\n-typedef int (*func_tag)(const char *name, const char *ref,\n+typedef int (*each_tag_name_fn)(const char *name, const char *ref,\n \t\t\t\tconst unsigned char *sha1);\n\n-static int do_tag_names(const char **argv, func_tag fn)\n+static int for_each_tag_name(const char **argv, each_tag_name_fn fn)\n {\n \tconst char **p;\n \tchar ref[PATH_MAX];\n@@ -191,7 +196,7 @@ static ssize_t do_sign(char *buffer, size_t size, size_t max)\n\n \tif (!*signingkey) {\n \t\tif (strlcpy(signingkey, git_committer_info(1),\n-\t\t\t\tsizeof(signingkey)) >= sizeof(signingkey))\n+\t\t\t\tsizeof(signingkey)) > sizeof(signingkey) - 1)\n \t\t\treturn error(\"committer info too long.\");\n \t\tbracket = strchr(signingkey, '>');\n \t\tif (bracket)\n@@ -254,7 +259,7 @@ static void create_tag(const unsigned char *object, const char *tag,\n \tunsigned long size;\n\n \ttype = sha1_object_info(object, NULL);\n-\tif (type <= 0)\n+\tif (type <= OBJ_NONE)\n \t    die(\"bad object type.\");\n\n \theader_len = snprintf(header_buf, sizeof(header_buf),\n@@ -267,7 +272,7 @@ static void create_tag(const unsigned char *object, const char *tag,\n \t\t\t  tag,\n \t\t\t  git_committer_info(1));\n\n-\tif (header_len >= sizeof(header_buf))\n+\tif (header_len > sizeof(header_buf) - 1)\n \t\tdie(\"tag header too big.\");\n\n \tif (!message) {\n@@ -362,6 +367,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\ti++;\n \t\t\tif (i == argc)\n \t\t\t\tdie(\"option -m needs an argument.\");\n+\t\t\tif (message)\n+\t\t\t\tdie(\"only one -F or -m option is allowed.\");\n \t\t\tmessage = xstrdup(argv[i]);\n \t\t\tcontinue;\n \t\t}\n@@ -373,6 +380,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\ti++;\n \t\t\tif (i == argc)\n \t\t\t\tdie(\"option -F needs an argument.\");\n+\t\t\tif (message)\n+\t\t\t\tdie(\"only one -F or -m option is allowed.\");\n\n \t\t\tif (!strcmp(argv[i], \"-\"))\n \t\t\t\tfd = 0;\n@@ -401,15 +410,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\t\tdie(\"argument to option -u too long\");\n \t\t\tcontinue;\n \t\t}\n-\t\tif (!strcmp(arg, \"-l\")) {\n+\t\tif (!strcmp(arg, \"-l\"))\n \t\t\treturn list_tags(argv[i + 1], lines);\n-\t\t}\n-\t\tif (!strcmp(arg, \"-d\")) {\n-\t\t\treturn do_tag_names(argv + i + 1, delete_tag);\n-\t\t}\n-\t\tif (!strcmp(arg, \"-v\")) {\n-\t\t\treturn do_tag_names(argv + i + 1, verify_tag);\n-\t\t}\n+\t\tif (!strcmp(arg, \"-d\"))\n+\t\t\treturn for_each_tag_name(argv + i + 1, delete_tag);\n+\t\tif (!strcmp(arg, \"-v\"))\n+\t\t\treturn for_each_tag_name(argv + i + 1, verify_tag);\n \t\tusage(builtin_tag_usage);\n \t}\n\n@@ -427,7 +433,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (get_sha1(object_ref, object))\n \t\tdie(\"Failed to resolve '%s' as a valid ref.\", object_ref);\n\n-\tif (snprintf(ref, sizeof(ref), \"refs/tags/%s\", tag) >= sizeof(ref))\n+\tif (snprintf(ref, sizeof(ref), \"refs/tags/%s\", tag) > sizeof(ref) - 1)\n \t\tdie(\"tag name too long: %.*s...\", 50, tag);\n \tif (check_ref_format(ref))\n \t\tdie(\"'%s' is not a valid tag name.\", tag);\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex a0be164..c4fa446 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -332,6 +332,33 @@ test_expect_success 'creating an annotated tag with -F - should succeed' '\n \tgit diff expect actual\n '\n\n+test_expect_success \\\n+\t'trying to create a tag with a non-existing -F file should fail' '\n+\t! test -f nonexistingfile &&\n+\t! tag_exists notag &&\n+\t! git-tag -F nonexistingfile notag &&\n+\t! tag_exists notag\n+'\n+\n+test_expect_success \\\n+\t'trying to create tags giving many -m or -F options should fail' '\n+\techo \"message file 1\" >msgfile1 &&\n+\techo \"message file 2\" >msgfile2 &&\n+\t! tag_exists msgtag &&\n+\t! git-tag -m \"message 1\" -m \"message 2\" msgtag &&\n+\t! tag_exists msgtag &&\n+\t! git-tag -F msgfile1 -F msgfile2 msgtag &&\n+\t! tag_exists msgtag &&\n+\t! git-tag -m \"message 1\" -F msgfile1 msgtag &&\n+\t! tag_exists msgtag &&\n+\t! git-tag -F msgfile1 -m \"message 1\" msgtag &&\n+\t! tag_exists msgtag &&\n+\t! git-tag -F msgfile1 -m \"message 1\" -F msgfile2 msgtag &&\n+\t! tag_exists msgtag &&\n+\t! git-tag -m \"message 1\" -F msgfile1 -m \"message 2\" msgtag &&\n+\t! tag_exists msgtag\n+'\n+\n # blank and empty messages:\n\n get_tag_header empty-annotated-tag $commit commit $time >expect\n@@ -648,6 +675,14 @@ test_expect_success 'creating a signed tag with -F - should succeed' '\n \tgit diff expect actual\n '\n\n+test_expect_success \\\n+\t'trying to create a signed tag with non-existing -F file should fail' '\n+\t! test -f nonexistingfile &&\n+\t! tag_exists nosigtag &&\n+\t! git-tag -s -F nonexistingfile nosigtag &&\n+\t! tag_exists nosigtag\n+'\n+\n test_expect_success 'verifying a signed tag should succeed' \\\n \t'git-tag -v signed-tag'\n\n-- \n1.5.0\n"},{"id":"48031","messageId":"46A1F98A.5040609@gmail.com","threadId":"9126","inReplyTo":"46A1F858.7080405@gmail.com","subject":"Re: [PATCH] builtin-tag.c: Fix two memory leaks and minor notation changes.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2007-07-21T12:18:18Z","receivedAt":"2007-07-21T12:18:18Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"Carlos Rica escribió:\n>   Those were commented in the responses to my previous patch:\n>   http://article.gmane.org/gmane.comp.version-control.git/53138\n\nA little error here when copying the link, the thread is:\nhttp://thread.gmane.org/gmane.comp.version-control.git/53036\n"},{"id":"48037","messageId":"Pine.LNX.4.64.0707211407450.14781@racer.site","threadId":"9126","inReplyTo":"46A1F858.7080405@gmail.com","subject":"Re: [PATCH] builtin-tag.c: Fix two memory leaks and minor notation changes.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-21T13:08:26Z","receivedAt":"2007-07-21T13:08:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 21 Jul 2007, Carlos Rica wrote:\n\n> A repeated call to read_sha1_file was not freing memory\n> when the buffer was allocated but returned size was zero.\n> \n> Also, now the program does not allow many -F or -m options,\n> which was a bug too because it was not freing the memory\n> allocated for any previous -F or -m options.\n> \n> Tests are provided for ensuring that only one option\n> -F or -m is given. Also, another test is shipped here,\n> to check that \"git tag\" fails when a non-existing file\n> is passed to the -F option, something that git-tag.sh\n> allowed creating the tag with an empty message.\n> \n> Signed-off-by: Carlos Rica <jasampler@gmail.com>\n\nThank you very much!\n\nAcked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n\nCiao,\nDscho\n"}]}