{"thread":{"id":"29873","subject":"[PATCH v3 0/3] Fix documented fixme's throughout","startedAt":"2012-03-07T22:21:24Z","lastAt":"2012-03-21T22:21:32Z","messageCount":9,"participants":["Jared Hance","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"186346","messageId":"cover.1331158240.git.jaredhance@gmail.com","threadId":"29873","inReplyTo":null,"subject":"[PATCH v3 0/3] Fix documented fixme's throughout","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2012-03-07T22:21:24Z","receivedAt":"2012-03-07T22:21:24Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"A few patches that (hopefully) don't change the behavior of git except to\nrectify a memory error. Also, this should ever so slightly help with\nthe parallelism of git (a GSoC proposal). All of these were found with\ncommented FIXME and git grep.\n\nJared Hance (3):\n  Fix memory leak in apply_patch in apply.c.\n  Add threaded versions of functions in symlinks.c.\n  Use startup_info->prefix rather than prefix.\n\n builtin/apply.c |   31 ++++++++++++++++++++++++++++---\n cache.h         |    4 +++-\n git.c           |    2 +-\n symlinks.c      |   28 ++++++++++++++++++++++++++--\n trace.c         |   10 +++++-----\n 5 files changed, 63 insertions(+), 12 deletions(-)\n\n-- \n\nMinor style changes and move the most controversial commit to the end of the\npatch series.\n\nSorry, accidently sent out an old saved version of this just a bit before.\n\n1.7.3.4\n"},{"id":"186347","messageId":"eadfc83a0d823cc04ea37bf606b57597fb632156.1331158240.git.jaredhance@gmail.com","threadId":"29873","inReplyTo":"cover.1331158240.git.jaredhance@gmail.com","subject":"[PATCH v3 1/3] Fix memory leak in apply_patch in apply.c.","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2012-03-07T22:21:25Z","receivedAt":"2012-03-07T22:21:25Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"In the while loop inside apply_patch, patch is dynamically allocated\nwith a calloc. However, only unused patches are actually free'd; the\nrest are left in a memory leak. Since a list is actively built up\nconsisting of the used patches, they can simply be iterated and free'd\nat the end of the function.\n\nIn addition, the list of fragments should be free'd. To fix this, the\nutility function free_patch has been implemented. It loops over the\nentire patch list, and in each patch, loops over the fragment list,\nfreeing the fragments, followed by the patch in the list. It frees both\npatch and patch->next.\n\nThe main caveat is that the text in a fragment, ie,\npatch->fragments->patch, may or may not need to be free'd. The text is\ndynamically allocated and needs to be freed iff the patch is a binary\npatch, as allocation occurs in inflate_it.\n\nSigned-off-by: Jared Hance <jaredhance@gmail.com>\n---\n builtin/apply.c |   31 ++++++++++++++++++++++++++++---\n 1 files changed, 28 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 389898f..4c6b278 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -153,6 +153,7 @@ struct fragment {\n \tunsigned long oldpos, oldlines;\n \tunsigned long newpos, newlines;\n \tconst char *patch;\n+\tunsigned int free_patch:1;\n \tint size;\n \tint rejected;\n \tint linenr;\n@@ -196,6 +197,29 @@ struct patch {\n \tstruct patch *next;\n };\n \n+static void free_patch(struct patch *patch)\n+{\n+\twhile (patch != NULL) {\n+\t\tstruct patch *patch_next;\n+\t\tstruct fragment *fragment;\n+\n+\t\tpatch_next = patch->next;\n+\n+\t\tfragment = patch->fragments;\n+\t\twhile (fragment != NULL) {\n+\t\t\tstruct fragment *fragment_next = fragment->next;\n+\t\t\tif (fragment->patch != NULL && fragment->free_patch) {\n+\t\t\t\tfree((void*) fragment->patch);\n+\t\t\t}\n+\t\t\tfree(fragment);\n+\t\t\tfragment = fragment_next;\n+\t\t}\n+\n+\t\tfree(patch);\n+\t\tpatch = patch_next;\n+\t}\n+}\n+\n /*\n  * A line in a file, len-bytes long (includes the terminating LF,\n  * except for an incomplete line at the end if the file ends with\n@@ -1742,6 +1766,7 @@ static struct fragment *parse_binary_hunk(char **buf_p,\n \n \tfrag = xcalloc(1, sizeof(*frag));\n \tfrag->patch = inflate_it(data, hunk_size, origlen);\n+\tfrag->free_patch = 1;\n \tif (!frag->patch)\n \t\tgoto corrupt;\n \tfree(data);\n@@ -3687,7 +3712,6 @@ static int apply_patch(int fd, const char *filename, int options)\n \tstruct patch *list = NULL, **listp = &list;\n \tint skipped_patch = 0;\n \n-\t/* FIXME - memory leak when using multiple patch files as inputs */\n \tmemset(&fn_table, 0, sizeof(struct string_list));\n \tpatch_input_file = filename;\n \tread_patch_file(&buf, fd);\n@@ -3712,8 +3736,7 @@ static int apply_patch(int fd, const char *filename, int options)\n \t\t\tlistp = &patch->next;\n \t\t}\n \t\telse {\n-\t\t\t/* perhaps free it a bit better? */\n-\t\t\tfree(patch);\n+\t\t\tfree_patch(patch);\n \t\t\tskipped_patch++;\n \t\t}\n \t\toffset += nr;\n@@ -3754,6 +3777,8 @@ static int apply_patch(int fd, const char *filename, int options)\n \tif (summary)\n \t\tsummary_patch_list(list);\n \n+\tfree_patch(list);\n+\n \tstrbuf_release(&buf);\n \treturn 0;\n }\n-- \n1.7.3.4\n"},{"id":"186348","messageId":"5acf65ff331b28196c9781c39d1d00c3f8a733a4.1331158240.git.jaredhance@gmail.com","threadId":"29873","inReplyTo":"cover.1331158240.git.jaredhance@gmail.com","subject":"[PATCH v3 2/3] Add threaded versions of functions in symlinks.c.","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2012-03-07T22:21:26Z","receivedAt":"2012-03-07T22:21:26Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"check_leading_path and has_dirs_only_path both always use the default\ncache, which could be a caveat for adding parallelism (which is a\nconcern and even a GSoC proposal). This patch implements\nthreaded_check_leading_path and threading threaded_has_dirs_only_path\nand then implements the nonthreaded functions in terms of their threaded\nequivalents. No functional should be changed.\n\nSigned-off-by: Jared Hance <jaredhance@gmail.com>\n---\n cache.h    |    2 ++\n symlinks.c |   28 ++++++++++++++++++++++++++--\n 2 files changed, 28 insertions(+), 2 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex e12b15f..e5e1aa4 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -950,7 +950,9 @@ struct cache_def {\n extern int has_symlink_leading_path(const char *name, int len);\n extern int threaded_has_symlink_leading_path(struct cache_def *, const char *, int);\n extern int check_leading_path(const char *name, int len);\n+extern int threaded_check_leading_path(struct cache_def *cache, const char *name, int len);\n extern int has_dirs_only_path(const char *name, int len, int prefix_len);\n+extern int threaded_has_dirs_only_path(struct cache_def *cache, const char *name, int len, int prefix_len);\n extern void schedule_dir_for_removal(const char *name, int len);\n extern void remove_scheduled_dirs(void);\n \ndiff --git a/symlinks.c b/symlinks.c\nindex 034943b..2900367 100644\n--- a/symlinks.c\n+++ b/symlinks.c\n@@ -219,7 +219,20 @@ int has_symlink_leading_path(const char *name, int len)\n  */\n int check_leading_path(const char *name, int len)\n {\n-\tstruct cache_def *cache = &default_cache;\t/* FIXME */\n+    return threaded_check_leading_path(&default_cache, name, len);\n+}\n+\n+/*\n+ * Return zero if path 'name' has a leading symlink component or\n+ * if some leading path component does not exists.\n+ *\n+ * Return -1 if leading path exists and is a directory.\n+ *\n+ * Return path length if leading path exists and is neither a\n+ * directory nor a symlink.\n+ */\n+int threaded_check_leading_path(struct cache_def *cache, const char *name, int len)\n+{\n \tint flags;\n \tint match_len = lstat_cache_matchlen(cache, name, len, &flags,\n \t\t\t   FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT);\n@@ -240,7 +253,18 @@ int check_leading_path(const char *name, int len)\n  */\n int has_dirs_only_path(const char *name, int len, int prefix_len)\n {\n-\tstruct cache_def *cache = &default_cache;\t/* FIXME */\n+\treturn threaded_has_dirs_only_path(&default_cache, name, len, prefix_len);\n+}\n+\n+/*\n+ * Return non-zero if all path components of 'name' exists as a\n+ * directory.  If prefix_len > 0, we will test with the stat()\n+ * function instead of the lstat() function for a prefix length of\n+ * 'prefix_len', thus we then allow for symlinks in the prefix part as\n+ * long as those points to real existing directories.\n+ */\n+int threaded_has_dirs_only_path(struct cache_def *cache, const char *name, int len, int prefix_len)\n+{\n \treturn lstat_cache(cache, name, len,\n \t\t\t   FL_DIR|FL_FULLPATH, prefix_len) &\n \t\tFL_DIR;\n-- \n1.7.3.4\n"},{"id":"186349","messageId":"a2980b437f2eb81b6bc28ad4ebbf05429485729e.1331158240.git.jaredhance@gmail.com","threadId":"29873","inReplyTo":"cover.1331158240.git.jaredhance@gmail.com","subject":"[PATCH v3 3/3] Use startup_info->prefix rather than prefix.","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2012-03-07T22:21:27Z","receivedAt":"2012-03-07T22:21:27Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"In trace_repo_setup, prefix is passed in as startup_info->prefix. But, as\nindicated but a FIXME comment, trace_repo_setup has access to\nstartup_info. The prefix parameter has therefore been eliminated.\n\nSigned-off-by: Jared Hance <jaredhance@gmail.com>\n---\n cache.h |    2 +-\n git.c   |    2 +-\n trace.c |   10 +++++-----\n 3 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex e5e1aa4..1113296 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1213,7 +1213,7 @@ extern void trace_printf(const char *format, ...);\n extern void trace_vprintf(const char *key, const char *format, va_list ap);\n __attribute__((format (printf, 2, 3)))\n extern void trace_argv_printf(const char **argv, const char *format, ...);\n-extern void trace_repo_setup(const char *prefix);\n+extern void trace_repo_setup(void);\n extern int trace_want(const char *key);\n extern void trace_strbuf(const char *key, const struct strbuf *buf);\n \ndiff --git a/git.c b/git.c\nindex 3805616..7dcc527 100644\n--- a/git.c\n+++ b/git.c\n@@ -296,7 +296,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)\n \n \t\tif ((p->option & (RUN_SETUP | RUN_SETUP_GENTLY)) &&\n \t\t    startup_info->have_repository) /* get_git_dir() may set up repo, avoid that */\n-\t\t\ttrace_repo_setup(prefix);\n+\t\t\ttrace_repo_setup();\n \t}\n \tcommit_pager_choice();\n \ndiff --git a/trace.c b/trace.c\nindex d953416..09a470b 100644\n--- a/trace.c\n+++ b/trace.c\n@@ -152,8 +152,7 @@ static const char *quote_crnl(const char *path)\n \treturn new_path;\n }\n \n-/* FIXME: move prefix to startup_info struct and get rid of this arg */\n-void trace_repo_setup(const char *prefix)\n+void trace_repo_setup(void)\n {\n \tstatic const char *key = \"GIT_TRACE_SETUP\";\n \tconst char *git_work_tree;\n@@ -168,13 +167,14 @@ void trace_repo_setup(const char *prefix)\n \tif (!(git_work_tree = get_git_work_tree()))\n \t\tgit_work_tree = \"(null)\";\n \n-\tif (!prefix)\n-\t\tprefix = \"(null)\";\n+\tif (!startup_info->prefix)\n+\t\tstartup_info->prefix = \"(null)\";\n \n \ttrace_printf_key(key, \"setup: git_dir: %s\\n\", quote_crnl(get_git_dir()));\n \ttrace_printf_key(key, \"setup: worktree: %s\\n\", quote_crnl(git_work_tree));\n \ttrace_printf_key(key, \"setup: cwd: %s\\n\", quote_crnl(cwd));\n-\ttrace_printf_key(key, \"setup: prefix: %s\\n\", quote_crnl(prefix));\n+\ttrace_printf_key(key, \"setup: prefix: %s\\n\",\n+\t\t\t quote_crnl(startup_info->prefix));\n }\n \n int trace_want(const char *key)\n-- \n1.7.3.4\n"},{"id":"186358","messageId":"7v62egkmlb.fsf@alter.siamese.dyndns.org","threadId":"29873","inReplyTo":"5acf65ff331b28196c9781c39d1d00c3f8a733a4.1331158240.git.jaredhance@gmail.com","subject":"Re: [PATCH v3 2/3] Add threaded versions of functions in symlinks.c.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-07T23:39:12Z","receivedAt":"2012-03-07T23:39:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is identical to what is listed as \"Cooking already in 'next'\"\nin the latest issue of \"What's cooking\", and I've already merged it\nto 'master' in preparation for 1.7.10-rc0.\n\nThanks.\n"},{"id":"186359","messageId":"7vy5rcj806.fsf@alter.siamese.dyndns.org","threadId":"29873","inReplyTo":"a2980b437f2eb81b6bc28ad4ebbf05429485729e.1331158240.git.jaredhance@gmail.com","subject":"Re: [PATCH v3 3/3] Use startup_info->prefix rather than prefix.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-07T23:39:37Z","receivedAt":"2012-03-07T23:39:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nothing seems to have changed in this patch since the last round.\n\nMy impression has been that we agreed with your comment\n\n    Note: I'm not quite sure if I actually agree with the first change. It makes\n    sense right now if git.c is the only caller, but in the future, it might become\n    less flexible.\n\nand decided to drop this patch at least for now.\n"},{"id":"186360","messageId":"7vr4x4j800.fsf@alter.siamese.dyndns.org","threadId":"29873","inReplyTo":"eadfc83a0d823cc04ea37bf606b57597fb632156.1331158240.git.jaredhance@gmail.com","subject":"Re: [PATCH v3 1/3] Fix memory leak in apply_patch in apply.c.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-07T23:39:43Z","receivedAt":"2012-03-07T23:39:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jared Hance <jaredhance@gmail.com> writes:\n\n> In the while loop inside apply_patch, patch is dynamically allocated\n> with a calloc. However, only unused patches are actually free'd; the\n> rest are left in a memory leak. Since a list is actively built up\n> consisting of the used patches, they can simply be iterated and free'd\n> at the end of the function.\n> ...\n\nThanks.\n\nThis more-or-less looks good modulo minor style issues.  We might\nalso want to make rejected a one-bit bitfield that sits next to the\nnew free_patch field to share the same word, but that is a separate\ntopic.\n\nWill queue.\n"},{"id":"187438","messageId":"7v1uol1tud.fsf_-_@alter.siamese.dyndns.org","threadId":"29873","inReplyTo":"eadfc83a0d823cc04ea37bf606b57597fb632156.1331158240.git.jaredhance@gmail.com","subject":"[PATCH 1/2] apply: free patch->{def,old,new}_name fields","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-21T22:18:18Z","receivedAt":"2012-03-21T22:18:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"These were all allocated in the heap by parsing the header parts of the\npatch, but we did not bother to free them.  Some used to share the memory\n(e.g. copying def_name to old_name) so this is not just the matter of\nadding three calls to free().\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/apply.c |   65 +++++++++++++++++++++++++++++++++----------------------\n 1 file changed, 39 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 427c263..5d03e50 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -209,6 +209,9 @@ static void free_patch(struct patch *patch)\n \t\t\tfree(fragment);\n \t\t\tfragment = fragment_next;\n \t\t}\n+\t\tfree(patch->def_name);\n+\t\tfree(patch->old_name);\n+\t\tfree(patch->new_name);\n \t\tfree(patch);\n \t\tpatch = patch_next;\n \t}\n@@ -434,7 +437,7 @@ static char *squash_slash(char *name)\n \treturn name;\n }\n \n-static char *find_name_gnu(const char *line, char *def, int p_value)\n+static char *find_name_gnu(const char *line, const char *def, int p_value)\n {\n \tstruct strbuf name = STRBUF_INIT;\n \tchar *cp;\n@@ -457,11 +460,7 @@ static char *find_name_gnu(const char *line, char *def, int p_value)\n \t\tcp++;\n \t}\n \n-\t/* name can later be freed, so we need\n-\t * to memmove, not just return cp\n-\t */\n \tstrbuf_remove(&name, 0, cp - name.buf);\n-\tfree(def);\n \tif (root)\n \t\tstrbuf_insert(&name, 0, root, root_len);\n \treturn squash_slash(strbuf_detach(&name, NULL));\n@@ -626,8 +625,13 @@ static size_t diff_timestamp_len(const char *line, size_t len)\n \treturn line + len - end;\n }\n \n-static char *find_name_common(const char *line, char *def, int p_value,\n-\t\t\t\tconst char *end, int terminate)\n+static char *null_strdup(const char *s)\n+{\n+\treturn s ? xstrdup(s) : NULL;\n+}\n+\n+static char *find_name_common(const char *line, const char *def,\n+\t\t\t      int p_value, const char *end, int terminate)\n {\n \tint len;\n \tconst char *start = NULL;\n@@ -648,10 +652,10 @@ static char *find_name_common(const char *line, char *def, int p_value,\n \t\t\tstart = line;\n \t}\n \tif (!start)\n-\t\treturn squash_slash(def);\n+\t\treturn squash_slash(null_strdup(def));\n \tlen = line - start;\n \tif (!len)\n-\t\treturn squash_slash(def);\n+\t\treturn squash_slash(null_strdup(def));\n \n \t/*\n \t * Generally we prefer the shorter name, especially\n@@ -662,8 +666,7 @@ static char *find_name_common(const char *line, char *def, int p_value,\n \tif (def) {\n \t\tint deflen = strlen(def);\n \t\tif (deflen < len && !strncmp(start, def, deflen))\n-\t\t\treturn squash_slash(def);\n-\t\tfree(def);\n+\t\t\treturn squash_slash(xstrdup(def));\n \t}\n \n \tif (root) {\n@@ -860,8 +863,10 @@ static void parse_traditional_patch(const char *first, const char *second, struc\n \t\tname = find_name_traditional(first, NULL, p_value);\n \t\tpatch->old_name = name;\n \t} else {\n-\t\tname = find_name_traditional(first, NULL, p_value);\n-\t\tname = find_name_traditional(second, name, p_value);\n+\t\tchar *first_name;\n+\t\tfirst_name = find_name_traditional(first, NULL, p_value);\n+\t\tname = find_name_traditional(second, first_name, p_value);\n+\t\tfree(first_name);\n \t\tif (has_epoch_timestamp(first)) {\n \t\t\tpatch->is_new = 1;\n \t\t\tpatch->is_delete = 0;\n@@ -871,7 +876,8 @@ static void parse_traditional_patch(const char *first, const char *second, struc\n \t\t\tpatch->is_delete = 1;\n \t\t\tpatch->old_name = name;\n \t\t} else {\n-\t\t\tpatch->old_name = patch->new_name = name;\n+\t\t\tpatch->old_name = name;\n+\t\t\tpatch->new_name = xstrdup(name);\n \t\t}\n \t}\n \tif (!name)\n@@ -921,13 +927,19 @@ static char *gitdiff_verify_name(const char *line, int isnull, char *orig_name,\n \n static int gitdiff_oldname(const char *line, struct patch *patch)\n {\n+\tchar *orig = patch->old_name;\n \tpatch->old_name = gitdiff_verify_name(line, patch->is_new, patch->old_name, \"old\");\n+\tif (orig != patch->old_name)\n+\t\tfree(orig);\n \treturn 0;\n }\n \n static int gitdiff_newname(const char *line, struct patch *patch)\n {\n+\tchar *orig = patch->new_name;\n \tpatch->new_name = gitdiff_verify_name(line, patch->is_delete, patch->new_name, \"new\");\n+\tif (orig != patch->new_name)\n+\t\tfree(orig);\n \treturn 0;\n }\n \n@@ -946,20 +958,23 @@ static int gitdiff_newmode(const char *line, struct patch *patch)\n static int gitdiff_delete(const char *line, struct patch *patch)\n {\n \tpatch->is_delete = 1;\n-\tpatch->old_name = patch->def_name;\n+\tfree(patch->old_name);\n+\tpatch->old_name = null_strdup(patch->def_name);\n \treturn gitdiff_oldmode(line, patch);\n }\n \n static int gitdiff_newfile(const char *line, struct patch *patch)\n {\n \tpatch->is_new = 1;\n-\tpatch->new_name = patch->def_name;\n+\tfree(patch->new_name);\n+\tpatch->new_name = null_strdup(patch->def_name);\n \treturn gitdiff_newmode(line, patch);\n }\n \n static int gitdiff_copysrc(const char *line, struct patch *patch)\n {\n \tpatch->is_copy = 1;\n+\tfree(patch->old_name);\n \tpatch->old_name = find_name(line, NULL, p_value ? p_value - 1 : 0, 0);\n \treturn 0;\n }\n@@ -967,6 +982,7 @@ static int gitdiff_copysrc(const char *line, struct patch *patch)\n static int gitdiff_copydst(const char *line, struct patch *patch)\n {\n \tpatch->is_copy = 1;\n+\tfree(patch->new_name);\n \tpatch->new_name = find_name(line, NULL, p_value ? p_value - 1 : 0, 0);\n \treturn 0;\n }\n@@ -974,6 +990,7 @@ static int gitdiff_copydst(const char *line, struct patch *patch)\n static int gitdiff_renamesrc(const char *line, struct patch *patch)\n {\n \tpatch->is_rename = 1;\n+\tfree(patch->old_name);\n \tpatch->old_name = find_name(line, NULL, p_value ? p_value - 1 : 0, 0);\n \treturn 0;\n }\n@@ -981,6 +998,7 @@ static int gitdiff_renamesrc(const char *line, struct patch *patch)\n static int gitdiff_renamedst(const char *line, struct patch *patch)\n {\n \tpatch->is_rename = 1;\n+\tfree(patch->new_name);\n \tpatch->new_name = find_name(line, NULL, p_value ? p_value - 1 : 0, 0);\n \treturn 0;\n }\n@@ -1421,7 +1439,8 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc\n \t\t\t\tif (!patch->def_name)\n \t\t\t\t\tdie(\"git diff header lacks filename information when removing \"\n \t\t\t\t\t    \"%d leading pathname components (line %d)\" , p_value, linenr);\n-\t\t\t\tpatch->old_name = patch->new_name = patch->def_name;\n+\t\t\t\tpatch->old_name = xstrdup(patch->def_name);\n+\t\t\t\tpatch->new_name = xstrdup(patch->def_name);\n \t\t\t}\n \t\t\tif (!patch->is_delete && !patch->new_name)\n \t\t\t\tdie(\"git diff header lacks filename information \"\n@@ -3104,6 +3123,7 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s\n  is_new:\n \tpatch->is_new = 1;\n \tpatch->is_delete = 0;\n+\tfree(patch->old_name);\n \tpatch->old_name = NULL;\n \treturn 0;\n }\n@@ -3684,15 +3704,8 @@ static void prefix_patches(struct patch *p)\n \tif (!prefix || p->is_toplevel_relative)\n \t\treturn;\n \tfor ( ; p; p = p->next) {\n-\t\tif (p->new_name == p->old_name) {\n-\t\t\tchar *prefixed = p->new_name;\n-\t\t\tprefix_one(&prefixed);\n-\t\t\tp->new_name = p->old_name = prefixed;\n-\t\t}\n-\t\telse {\n-\t\t\tprefix_one(&p->new_name);\n-\t\t\tprefix_one(&p->old_name);\n-\t\t}\n+\t\tprefix_one(&p->new_name);\n+\t\tprefix_one(&p->old_name);\n \t}\n }\n \n-- \n1.7.10.rc1.76.g1a8310\n"},{"id":"187440","messageId":"7vwr6dzjbn.fsf_-_@alter.siamese.dyndns.org","threadId":"29873","inReplyTo":"7v1uol1tud.fsf_-_@alter.siamese.dyndns.org","subject":"[PATCH 2/2] apply: free patch->result","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-21T22:21:32Z","receivedAt":"2012-03-21T22:21:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is by far the largest piece of data, much larger than the patch and\nfragment structures or the three name fields in the patch structure.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\nI have not finished auditing all the codepaths, so this change needs to be\neyeballed carefully. We may be pointing an unfreeable piece of memory or a\npiece of memory that belong to other structures that are going to be freed\notherwise.\n\n builtin/apply.c |    1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 5d03e50..c919db3 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -212,6 +212,7 @@ static void free_patch(struct patch *patch)\n \t\tfree(patch->def_name);\n \t\tfree(patch->old_name);\n \t\tfree(patch->new_name);\n+\t\tfree(patch->result);\n \t\tfree(patch);\n \t\tpatch = patch_next;\n \t}\n-- \n1.7.10.rc1.76.g1a8310\n"}]}