{"thread":{"id":"33985","subject":"[PATCH 0/7] unpack-trees: plug memory leak for merges","startedAt":"2013-05-30T11:34:17Z","lastAt":"2013-06-04T15:06:08Z","messageCount":12,"participants":["René Scharfe","Felipe Contreras","Peter Krefting"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"218967","messageId":"1369913664-49734-1-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"33985","inReplyTo":null,"subject":"[PATCH 0/7] unpack-trees: plug memory leak for merges","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-30T11:34:17Z","receivedAt":"2013-05-30T11:34:17Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"This series adds const to cache_entry pointers in a lot of places, in\norder to show that we can free them in unpack_nondirectories, which\nthe last patch finally does.\n\nFirst three easy patches for adding const and splitting a function in\ntwo:\n\n  cache: mark cache_entry pointers const\n  read-cache: mark cache_entry pointers const\n  unpack-trees: factor out dup_entry\n\nThen a patch that reduces the side effects of merged_entry:\n\n  unpack-trees: create working copy of merge entry in merged_entry\n\nAnother easy const patch:\n\n  diff-lib, read-tree, unpack-trees: mark cache_entry pointers const\n\nAnd patch that introduces \"const struct cache_entry * const *\", which\nmay look a bit scary at first:\n\n  diff-lib, read-tree, unpack-trees: mark cache_entry array paramters const\n\nFinal patch that plugs a memory leak in unpack_nondirectories.\n\n  unpack-trees: free cache_entry array members for merges\n\nIt's basically the same one that Stephen tested a while ago\n(http://thread.gmane.org/gmane.comp.version-control.git/222887).\n\nIt's not the only leak that affects cherry-pick; expect more\n(independent) patches.\n\n builtin/read-tree.c |   5 +-\n cache.h             |  10 ++--\n diff-lib.c          |  26 +++++-----\n read-cache.c        |  18 ++++---\n unpack-trees.c      | 146 ++++++++++++++++++++++++++++++++--------------------\n unpack-trees.h      |  14 +++--\n 6 files changed, 131 insertions(+), 88 deletions \n1.8.3\n"},{"id":"218968","messageId":"1369913664-49734-2-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"33985","inReplyTo":"1369913664-49734-1-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 1/7] cache: mark cache_entry pointers const","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-30T11:34:18Z","receivedAt":"2013-05-30T11:34:18Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Add const for pointers that are only dereferenced for reading by the\ninline functions copy_cache_entry and ce_mode_from_stat.  This allows\ncallers to pass in const pointers.\n\nSigned-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n cache.h | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 94ca1ac..43a27e7 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -190,7 +190,8 @@ struct cache_entry {\n  * another. But we never change the name, or the hash state!\n  */\n #define CE_STATE_MASK (CE_HASHED | CE_UNHASHED)\n-static inline void copy_cache_entry(struct cache_entry *dst, struct cache_entry *src)\n+static inline void copy_cache_entry(struct cache_entry *dst,\n+\t\t\t\t    const struct cache_entry *src)\n {\n \tunsigned int state = dst->ce_flags & CE_STATE_MASK;\n \n@@ -222,7 +223,8 @@ static inline unsigned int create_ce_mode(unsigned int mode)\n \t\treturn S_IFGITLINK;\n \treturn S_IFREG | ce_permissions(mode);\n }\n-static inline unsigned int ce_mode_from_stat(struct cache_entry *ce, unsigned int mode)\n+static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,\n+\t\t\t\t\t     unsigned int mode)\n {\n \textern int trust_executable_bit, has_symlinks;\n \tif (!has_symlinks && S_ISREG(mode) &&\n-- \n1.8.3\n"},{"id":"218969","messageId":"1369913664-49734-3-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"33985","inReplyTo":"1369913664-49734-1-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 2/7] read-cache: mark cache_entry pointers const","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-30T11:34:19Z","receivedAt":"2013-05-30T11:34:19Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"ie_match_stat and ie_modified only derefence their struct cache_entry\npointers for reading.  Add const to the parameter declaration here and\ndo the same for the static helper function used by them, as it's the\nsame there as well.  This allows callers to pass in const pointers.\n\nSigned-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n cache.h      |  4 ++--\n read-cache.c | 18 ++++++++++--------\n 2 files changed, 12 insertions(+), 10 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 43a27e7..01e8760 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -482,8 +482,8 @@ extern void *read_blob_data_from_index(struct index_state *, const char *, unsig\n #define CE_MATCH_RACY_IS_DIRTY\t\t02\n /* do stat comparison even if CE_SKIP_WORKTREE is true */\n #define CE_MATCH_IGNORE_SKIP_WORKTREE\t04\n-extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);\n-extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);\n+extern int ie_match_stat(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int);\n+extern int ie_modified(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int);\n \n #define PATHSPEC_ONESTAR 1\t/* the pathspec pattern sastisfies GFNM_ONESTAR */\n \ndiff --git a/read-cache.c b/read-cache.c\nindex 04ed561..e6e0466 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -91,7 +91,7 @@ void fill_stat_cache_info(struct cache_entry *ce, struct stat *st)\n \t\tce_mark_uptodate(ce);\n }\n \n-static int ce_compare_data(struct cache_entry *ce, struct stat *st)\n+static int ce_compare_data(const struct cache_entry *ce, struct stat *st)\n {\n \tint match = -1;\n \tint fd = open(ce->name, O_RDONLY);\n@@ -105,7 +105,7 @@ static int ce_compare_data(struct cache_entry *ce, struct stat *st)\n \treturn match;\n }\n \n-static int ce_compare_link(struct cache_entry *ce, size_t expected_size)\n+static int ce_compare_link(const struct cache_entry *ce, size_t expected_size)\n {\n \tint match = -1;\n \tvoid *buffer;\n@@ -126,7 +126,7 @@ static int ce_compare_link(struct cache_entry *ce, size_t expected_size)\n \treturn match;\n }\n \n-static int ce_compare_gitlink(struct cache_entry *ce)\n+static int ce_compare_gitlink(const struct cache_entry *ce)\n {\n \tunsigned char sha1[20];\n \n@@ -143,7 +143,7 @@ static int ce_compare_gitlink(struct cache_entry *ce)\n \treturn hashcmp(sha1, ce->sha1);\n }\n \n-static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)\n+static int ce_modified_check_fs(const struct cache_entry *ce, struct stat *st)\n {\n \tswitch (st->st_mode & S_IFMT) {\n \tcase S_IFREG:\n@@ -163,7 +163,7 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)\n \treturn 0;\n }\n \n-static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n+static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)\n {\n \tunsigned int changed = 0;\n \n@@ -239,7 +239,8 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n \treturn changed;\n }\n \n-static int is_racy_timestamp(const struct index_state *istate, struct cache_entry *ce)\n+static int is_racy_timestamp(const struct index_state *istate,\n+\t\t\t     const struct cache_entry *ce)\n {\n \treturn (!S_ISGITLINK(ce->ce_mode) &&\n \t\tistate->timestamp.sec &&\n@@ -255,7 +256,7 @@ static int is_racy_timestamp(const struct index_state *istate, struct cache_entr\n }\n \n int ie_match_stat(const struct index_state *istate,\n-\t\t  struct cache_entry *ce, struct stat *st,\n+\t\t  const struct cache_entry *ce, struct stat *st,\n \t\t  unsigned int options)\n {\n \tunsigned int changed;\n@@ -311,7 +312,8 @@ int ie_match_stat(const struct index_state *istate,\n }\n \n int ie_modified(const struct index_state *istate,\n-\t\tstruct cache_entry *ce, struct stat *st, unsigned int options)\n+\t\tconst struct cache_entry *ce,\n+\t\tstruct stat *st, unsigned int options)\n {\n \tint changed, changed_fs;\n \n-- \n1.8.3\n"},{"id":"218971","messageId":"1369913664-49734-4-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"33985","inReplyTo":"1369913664-49734-1-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 3/7] unpack-trees: factor out dup_entry","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-30T11:34:20Z","receivedAt":"2013-05-30T11:34:20Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"While we're add it, mark the struct cache_entry pointer of add_entry\nconst because we only read from it and this allows callers to pass in\nconst pointers.\n\nSigned-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n unpack-trees.c | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex ede4299..e8b4cc1 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -116,14 +116,20 @@ static void do_add_entry(struct unpack_trees_options *o, struct cache_entry *ce,\n \t\t\tADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);\n }\n \n-static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,\n-\tunsigned int set, unsigned int clear)\n+static struct cache_entry *dup_entry(const struct cache_entry *ce)\n {\n \tunsigned int size = ce_size(ce);\n \tstruct cache_entry *new = xmalloc(size);\n \n \tmemcpy(new, ce, size);\n-\tdo_add_entry(o, new, set, clear);\n+\treturn new;\n+}\n+\n+static void add_entry(struct unpack_trees_options *o,\n+\t\t      const struct cache_entry *ce,\n+\t\t      unsigned int set, unsigned int clear)\n+{\n+\tdo_add_entry(o, dup_entry(ce), set, clear);\n }\n \n /*\n-- \n1.8.3\n"},{"id":"218972","messageId":"1369913664-49734-5-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"33985","inReplyTo":"1369913664-49734-1-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 4/7] unpack-trees: create working copy of merge entry in merged_entry","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-30T11:34:21Z","receivedAt":"2013-05-30T11:34:21Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Duplicate the merge entry right away and work with that instead of\nmodifying the entry we got and duplicating it only at the end of\nthe function.  Then mark that pointer const to document that we\ndon't modify the referenced cache_entry.\n\nThis change is safe because all existing merge functions call\nmerged_entry just before returning (or not at all), i.e. they don't\ncare about changes to the referenced cache_entry after the call.\nunpack_nondirectories and unpack_index_entry, which call the merge\nfunctions through call_unpack_fn, aren't interested in such changes\nneither.\n\nThe change complicates merged_entry a bit because we have to free the\ncopy if we error out, but allows callers to pass a const pointer.\n\nSigned-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n unpack-trees.c | 17 ++++++++++++-----\n 1 file changed, 12 insertions(+), 5 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex e8b4cc1..2fecef8 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1466,10 +1466,12 @@ static int verify_absent_sparse(struct cache_entry *ce,\n \treturn verify_absent_1(ce, orphaned_error, o);\n }\n \n-static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n-\t\tstruct unpack_trees_options *o)\n+static int merged_entry(const struct cache_entry *ce,\n+\t\t\tstruct cache_entry *old,\n+\t\t\tstruct unpack_trees_options *o)\n {\n \tint update = CE_UPDATE;\n+\tstruct cache_entry *merge = dup_entry(ce);\n \n \tif (!old) {\n \t\t/*\n@@ -1487,8 +1489,11 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n \t\tupdate |= CE_ADDED;\n \t\tmerge->ce_flags |= CE_NEW_SKIP_WORKTREE;\n \n-\t\tif (verify_absent(merge, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))\n+\t\tif (verify_absent(merge,\n+\t\t\t\t  ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o)) {\n+\t\t\tfree(merge);\n \t\t\treturn -1;\n+\t\t}\n \t\tinvalidate_ce_path(merge, o);\n \t} else if (!(old->ce_flags & CE_CONFLICTED)) {\n \t\t/*\n@@ -1502,8 +1507,10 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n \t\t\tcopy_cache_entry(merge, old);\n \t\t\tupdate = 0;\n \t\t} else {\n-\t\t\tif (verify_uptodate(old, o))\n+\t\t\tif (verify_uptodate(old, o)) {\n+\t\t\t\tfree(merge);\n \t\t\t\treturn -1;\n+\t\t\t}\n \t\t\t/* Migrate old flags over */\n \t\t\tupdate |= old->ce_flags & (CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);\n \t\t\tinvalidate_ce_path(old, o);\n@@ -1516,7 +1523,7 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n \t\tinvalidate_ce_path(old, o);\n \t}\n \n-\tadd_entry(o, merge, update, CE_STAGEMASK);\n+\tdo_add_entry(o, merge, update, CE_STAGEMASK);\n \treturn 1;\n }\n \n-- \n1.8.3\n"},{"id":"218973","messageId":"1369913664-49734-6-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"33985","inReplyTo":"1369913664-49734-1-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 5/7] diff-lib, read-tree, unpack-trees: mark cache_entry pointers const","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-30T11:34:22Z","receivedAt":"2013-05-30T11:34:22Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Add const to struct cache_entry pointers throughout the tree which are\nonly used for reading.  This allows callers to pass in const pointers.\n\nSigned-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n builtin/read-tree.c |  2 +-\n diff-lib.c          | 23 +++++++-------\n unpack-trees.c      | 91 +++++++++++++++++++++++++++++------------------------\n 3 files changed, 63 insertions(+), 53 deletions(-)\n\ndiff --git a/builtin/read-tree.c b/builtin/read-tree.c\nindex 042ac1b..b847486 100644\n--- a/builtin/read-tree.c\n+++ b/builtin/read-tree.c\n@@ -66,7 +66,7 @@ static int exclude_per_directory_cb(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n-static void debug_stage(const char *label, struct cache_entry *ce,\n+static void debug_stage(const char *label, const struct cache_entry *ce,\n \t\t\tstruct unpack_trees_options *o)\n {\n \tprintf(\"%s \", label);\ndiff --git a/diff-lib.c b/diff-lib.c\nindex f35de0f..83d0cb8 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -64,8 +64,9 @@ static int check_removed(const struct cache_entry *ce, struct stat *st)\n  * commits, untracked content and/or modified content).\n  */\n static int match_stat_with_submodule(struct diff_options *diffopt,\n-\t\t\t\t      struct cache_entry *ce, struct stat *st,\n-\t\t\t\t      unsigned ce_option, unsigned *dirty_submodule)\n+\t\t\t\t     const struct cache_entry *ce,\n+\t\t\t\t     struct stat *st, unsigned ce_option,\n+\t\t\t\t     unsigned *dirty_submodule)\n {\n \tint changed = ce_match_stat(ce, st, ce_option);\n \tif (S_ISGITLINK(ce->ce_mode)) {\n@@ -237,7 +238,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n /* A file entry went away or appeared */\n static void diff_index_show_file(struct rev_info *revs,\n \t\t\t\t const char *prefix,\n-\t\t\t\t struct cache_entry *ce,\n+\t\t\t\t const struct cache_entry *ce,\n \t\t\t\t const unsigned char *sha1, int sha1_valid,\n \t\t\t\t unsigned int mode,\n \t\t\t\t unsigned dirty_submodule)\n@@ -246,7 +247,7 @@ static void diff_index_show_file(struct rev_info *revs,\n \t\t       sha1, sha1_valid, ce->name, dirty_submodule);\n }\n \n-static int get_stat_data(struct cache_entry *ce,\n+static int get_stat_data(const struct cache_entry *ce,\n \t\t\t const unsigned char **sha1p,\n \t\t\t unsigned int *modep,\n \t\t\t int cached, int match_missing,\n@@ -283,7 +284,7 @@ static int get_stat_data(struct cache_entry *ce,\n }\n \n static void show_new_file(struct rev_info *revs,\n-\t\t\t  struct cache_entry *new,\n+\t\t\t  const struct cache_entry *new,\n \t\t\t  int cached, int match_missing)\n {\n \tconst unsigned char *sha1;\n@@ -302,8 +303,8 @@ static void show_new_file(struct rev_info *revs,\n }\n \n static int show_modified(struct rev_info *revs,\n-\t\t\t struct cache_entry *old,\n-\t\t\t struct cache_entry *new,\n+\t\t\t const struct cache_entry *old,\n+\t\t\t const struct cache_entry *new,\n \t\t\t int report_missing,\n \t\t\t int cached, int match_missing)\n {\n@@ -362,8 +363,8 @@ static int show_modified(struct rev_info *revs,\n  * give you the position and number of entries in the index).\n  */\n static void do_oneway_diff(struct unpack_trees_options *o,\n-\tstruct cache_entry *idx,\n-\tstruct cache_entry *tree)\n+\t\t\t   const struct cache_entry *idx,\n+\t\t\t   const struct cache_entry *tree)\n {\n \tstruct rev_info *revs = o->unpack_data;\n \tint match_missing, cached;\n@@ -425,8 +426,8 @@ static void do_oneway_diff(struct unpack_trees_options *o,\n  */\n static int oneway_diff(struct cache_entry **src, struct unpack_trees_options *o)\n {\n-\tstruct cache_entry *idx = src[0];\n-\tstruct cache_entry *tree = src[1];\n+\tconst struct cache_entry *idx = src[0];\n+\tconst struct cache_entry *tree = src[1];\n \tstruct rev_info *revs = o->unpack_data;\n \n \t/*\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 2fecef8..c5a40df 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -241,8 +241,11 @@ static int check_updates(struct unpack_trees_options *o)\n \treturn errs != 0;\n }\n \n-static int verify_uptodate_sparse(struct cache_entry *ce, struct unpack_trees_options *o);\n-static int verify_absent_sparse(struct cache_entry *ce, enum unpack_trees_error_types, struct unpack_trees_options *o);\n+static int verify_uptodate_sparse(const struct cache_entry *ce,\n+\t\t\t\t  struct unpack_trees_options *o);\n+static int verify_absent_sparse(const struct cache_entry *ce,\n+\t\t\t\tenum unpack_trees_error_types,\n+\t\t\t\tstruct unpack_trees_options *o);\n \n static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_options *o)\n {\n@@ -326,7 +329,7 @@ static void mark_all_ce_unused(struct index_state *index)\n \t\tindex->cache[i]->ce_flags &= ~(CE_UNPACKED | CE_ADDED | CE_NEW_SKIP_WORKTREE);\n }\n \n-static int locate_in_src_index(struct cache_entry *ce,\n+static int locate_in_src_index(const struct cache_entry *ce,\n \t\t\t       struct unpack_trees_options *o)\n {\n \tstruct index_state *index = o->src_index;\n@@ -1001,7 +1004,9 @@ static void mark_new_skip_worktree(struct exclude_list *el,\n \t\t       select_flag, skip_wt_flag, el);\n }\n \n-static int verify_absent(struct cache_entry *, enum unpack_trees_error_types, struct unpack_trees_options *);\n+static int verify_absent(const struct cache_entry *,\n+\t\t\t enum unpack_trees_error_types,\n+\t\t\t struct unpack_trees_options *);\n /*\n  * N-way merge \"len\" trees.  Returns 0 on success, -1 on failure to manipulate the\n  * resulting index, -2 on failure to reflect the changes to the work tree.\n@@ -1171,12 +1176,13 @@ return_failed:\n \n /* Here come the merge functions */\n \n-static int reject_merge(struct cache_entry *ce, struct unpack_trees_options *o)\n+static int reject_merge(const struct cache_entry *ce,\n+\t\t\tstruct unpack_trees_options *o)\n {\n \treturn add_rejected_path(o, ERROR_WOULD_OVERWRITE, ce->name);\n }\n \n-static int same(struct cache_entry *a, struct cache_entry *b)\n+static int same(const struct cache_entry *a, const struct cache_entry *b)\n {\n \tif (!!a != !!b)\n \t\treturn 0;\n@@ -1193,9 +1199,9 @@ static int same(struct cache_entry *a, struct cache_entry *b)\n  * When a CE gets turned into an unmerged entry, we\n  * want it to be up-to-date\n  */\n-static int verify_uptodate_1(struct cache_entry *ce,\n-\t\t\t\t   struct unpack_trees_options *o,\n-\t\t\t\t   enum unpack_trees_error_types error_type)\n+static int verify_uptodate_1(const struct cache_entry *ce,\n+\t\t\t     struct unpack_trees_options *o,\n+\t\t\t     enum unpack_trees_error_types error_type)\n {\n \tstruct stat st;\n \n@@ -1234,7 +1240,7 @@ static int verify_uptodate_1(struct cache_entry *ce,\n \t\tadd_rejected_path(o, error_type, ce->name);\n }\n \n-static int verify_uptodate(struct cache_entry *ce,\n+static int verify_uptodate(const struct cache_entry *ce,\n \t\t\t   struct unpack_trees_options *o)\n {\n \tif (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))\n@@ -1242,13 +1248,14 @@ static int verify_uptodate(struct cache_entry *ce,\n \treturn verify_uptodate_1(ce, o, ERROR_NOT_UPTODATE_FILE);\n }\n \n-static int verify_uptodate_sparse(struct cache_entry *ce,\n+static int verify_uptodate_sparse(const struct cache_entry *ce,\n \t\t\t\t  struct unpack_trees_options *o)\n {\n \treturn verify_uptodate_1(ce, o, ERROR_SPARSE_NOT_UPTODATE_FILE);\n }\n \n-static void invalidate_ce_path(struct cache_entry *ce, struct unpack_trees_options *o)\n+static void invalidate_ce_path(const struct cache_entry *ce,\n+\t\t\t       struct unpack_trees_options *o)\n {\n \tif (ce)\n \t\tcache_tree_invalidate_path(o->src_index->cache_tree, ce->name);\n@@ -1261,16 +1268,16 @@ static void invalidate_ce_path(struct cache_entry *ce, struct unpack_trees_optio\n  * Currently, git does not checkout subprojects during a superproject\n  * checkout, so it is not going to overwrite anything.\n  */\n-static int verify_clean_submodule(struct cache_entry *ce,\n-\t\t\t\t      enum unpack_trees_error_types error_type,\n-\t\t\t\t      struct unpack_trees_options *o)\n+static int verify_clean_submodule(const struct cache_entry *ce,\n+\t\t\t\t  enum unpack_trees_error_types error_type,\n+\t\t\t\t  struct unpack_trees_options *o)\n {\n \treturn 0;\n }\n \n-static int verify_clean_subdirectory(struct cache_entry *ce,\n-\t\t\t\t      enum unpack_trees_error_types error_type,\n-\t\t\t\t      struct unpack_trees_options *o)\n+static int verify_clean_subdirectory(const struct cache_entry *ce,\n+\t\t\t\t     enum unpack_trees_error_types error_type,\n+\t\t\t\t     struct unpack_trees_options *o)\n {\n \t/*\n \t * we are about to extract \"ce->name\"; we would not want to lose\n@@ -1356,7 +1363,7 @@ static int icase_exists(struct unpack_trees_options *o, const char *name, int le\n }\n \n static int check_ok_to_remove(const char *name, int len, int dtype,\n-\t\t\t      struct cache_entry *ce, struct stat *st,\n+\t\t\t      const struct cache_entry *ce, struct stat *st,\n \t\t\t      enum unpack_trees_error_types error_type,\n \t\t\t      struct unpack_trees_options *o)\n {\n@@ -1411,9 +1418,9 @@ static int check_ok_to_remove(const char *name, int len, int dtype,\n  * We do not want to remove or overwrite a working tree file that\n  * is not tracked, unless it is ignored.\n  */\n-static int verify_absent_1(struct cache_entry *ce,\n-\t\t\t\t enum unpack_trees_error_types error_type,\n-\t\t\t\t struct unpack_trees_options *o)\n+static int verify_absent_1(const struct cache_entry *ce,\n+\t\t\t   enum unpack_trees_error_types error_type,\n+\t\t\t   struct unpack_trees_options *o)\n {\n \tint len;\n \tstruct stat st;\n@@ -1446,7 +1453,7 @@ static int verify_absent_1(struct cache_entry *ce,\n \t}\n }\n \n-static int verify_absent(struct cache_entry *ce,\n+static int verify_absent(const struct cache_entry *ce,\n \t\t\t enum unpack_trees_error_types error_type,\n \t\t\t struct unpack_trees_options *o)\n {\n@@ -1455,9 +1462,9 @@ static int verify_absent(struct cache_entry *ce,\n \treturn verify_absent_1(ce, error_type, o);\n }\n \n-static int verify_absent_sparse(struct cache_entry *ce,\n-\t\t\t enum unpack_trees_error_types error_type,\n-\t\t\t struct unpack_trees_options *o)\n+static int verify_absent_sparse(const struct cache_entry *ce,\n+\t\t\t\tenum unpack_trees_error_types error_type,\n+\t\t\t\tstruct unpack_trees_options *o)\n {\n \tenum unpack_trees_error_types orphaned_error = error_type;\n \tif (orphaned_error == ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN)\n@@ -1467,7 +1474,7 @@ static int verify_absent_sparse(struct cache_entry *ce,\n }\n \n static int merged_entry(const struct cache_entry *ce,\n-\t\t\tstruct cache_entry *old,\n+\t\t\tconst struct cache_entry *old,\n \t\t\tstruct unpack_trees_options *o)\n {\n \tint update = CE_UPDATE;\n@@ -1527,8 +1534,9 @@ static int merged_entry(const struct cache_entry *ce,\n \treturn 1;\n }\n \n-static int deleted_entry(struct cache_entry *ce, struct cache_entry *old,\n-\t\tstruct unpack_trees_options *o)\n+static int deleted_entry(const struct cache_entry *ce,\n+\t\t\t const struct cache_entry *old,\n+\t\t\t struct unpack_trees_options *o)\n {\n \t/* Did it exist in the index? */\n \tif (!old) {\n@@ -1543,7 +1551,8 @@ static int deleted_entry(struct cache_entry *ce, struct cache_entry *old,\n \treturn 1;\n }\n \n-static int keep_entry(struct cache_entry *ce, struct unpack_trees_options *o)\n+static int keep_entry(const struct cache_entry *ce,\n+\t\t      struct unpack_trees_options *o)\n {\n \tadd_entry(o, ce, 0, 0);\n \treturn 1;\n@@ -1567,9 +1576,9 @@ static void show_stage_entry(FILE *o,\n \n int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o)\n {\n-\tstruct cache_entry *index;\n-\tstruct cache_entry *head;\n-\tstruct cache_entry *remote = stages[o->head_idx + 1];\n+\tconst struct cache_entry *index;\n+\tconst struct cache_entry *head;\n+\tconst struct cache_entry *remote = stages[o->head_idx + 1];\n \tint count;\n \tint head_match = 0;\n \tint remote_match = 0;\n@@ -1654,7 +1663,7 @@ int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o)\n \tif (o->aggressive) {\n \t\tint head_deleted = !head;\n \t\tint remote_deleted = !remote;\n-\t\tstruct cache_entry *ce = NULL;\n+\t\tconst struct cache_entry *ce = NULL;\n \n \t\tif (index)\n \t\t\tce = index;\n@@ -1739,9 +1748,9 @@ int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o)\n  */\n int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n {\n-\tstruct cache_entry *current = src[0];\n-\tstruct cache_entry *oldtree = src[1];\n-\tstruct cache_entry *newtree = src[2];\n+\tconst struct cache_entry *current = src[0];\n+\tconst struct cache_entry *oldtree = src[1];\n+\tconst struct cache_entry *newtree = src[2];\n \n \tif (o->merge_size != 2)\n \t\treturn error(\"Cannot do a twoway merge of %d trees\",\n@@ -1806,8 +1815,8 @@ int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n int bind_merge(struct cache_entry **src,\n \t\tstruct unpack_trees_options *o)\n {\n-\tstruct cache_entry *old = src[0];\n-\tstruct cache_entry *a = src[1];\n+\tconst struct cache_entry *old = src[0];\n+\tconst struct cache_entry *a = src[1];\n \n \tif (o->merge_size != 1)\n \t\treturn error(\"Cannot do a bind merge of %d trees\",\n@@ -1829,8 +1838,8 @@ int bind_merge(struct cache_entry **src,\n  */\n int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n {\n-\tstruct cache_entry *old = src[0];\n-\tstruct cache_entry *a = src[1];\n+\tconst struct cache_entry *old = src[0];\n+\tconst struct cache_entry *a = src[1];\n \n \tif (o->merge_size != 1)\n \t\treturn error(\"Cannot do a oneway merge of %d trees\",\n-- \n1.8.3\n"},{"id":"218974","messageId":"1369913664-49734-7-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"33985","inReplyTo":"1369913664-49734-1-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 6/7] diff-lib, read-tree, unpack-trees: mark cache_entry array paramters const","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-30T11:34:23Z","receivedAt":"2013-05-30T11:34:23Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Change the type merge_fn_t to accept the array of cache_entry pointers\nas const pointers to const pointers.  This documents the fact that the\nmerge functions don't modify the cache_entry contents or replace any of\nthe pointers in the array.\n\nOnly a single cast is necessary in unpack_nondirectories because adding\ntwo const modifiers at once is not allowed in C.  The cast is safe in\nthat it doesn't mask any modfication; call_unpack_fn only needs the\narray for reading.\n\nSigned-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n builtin/read-tree.c |  3 ++-\n diff-lib.c          |  3 ++-\n unpack-trees.c      | 21 +++++++++++++--------\n unpack-trees.h      | 14 +++++++++-----\n 4 files changed, 26 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/read-tree.c b/builtin/read-tree.c\nindex b847486..0f5d7fe 100644\n--- a/builtin/read-tree.c\n+++ b/builtin/read-tree.c\n@@ -80,7 +80,8 @@ static void debug_stage(const char *label, const struct cache_entry *ce,\n \t\t       sha1_to_hex(ce->sha1));\n }\n \n-static int debug_merge(struct cache_entry **stages, struct unpack_trees_options *o)\n+static int debug_merge(const struct cache_entry * const *stages,\n+\t\t       struct unpack_trees_options *o)\n {\n \tint i;\n \ndiff --git a/diff-lib.c b/diff-lib.c\nindex 83d0cb8..b6f4b21 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -424,7 +424,8 @@ static void do_oneway_diff(struct unpack_trees_options *o,\n  * the fairly complex unpack_trees() semantic requirements, including\n  * the skipping, the path matching, the type conflict cases etc.\n  */\n-static int oneway_diff(struct cache_entry **src, struct unpack_trees_options *o)\n+static int oneway_diff(const struct cache_entry * const *src,\n+\t\t       struct unpack_trees_options *o)\n {\n \tconst struct cache_entry *idx = src[0];\n \tconst struct cache_entry *tree = src[1];\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex c5a40df..2dbc05d 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -300,7 +300,8 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt\n \treturn 0;\n }\n \n-static inline int call_unpack_fn(struct cache_entry **src, struct unpack_trees_options *o)\n+static inline int call_unpack_fn(const struct cache_entry * const *src,\n+\t\t\t\t struct unpack_trees_options *o)\n {\n \tint ret = o->fn(src, o);\n \tif (ret > 0)\n@@ -397,7 +398,7 @@ static void add_same_unmerged(struct cache_entry *ce,\n static int unpack_index_entry(struct cache_entry *ce,\n \t\t\t      struct unpack_trees_options *o)\n {\n-\tstruct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, };\n+\tconst struct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, };\n \tint ret;\n \n \tsrc[0] = ce;\n@@ -600,7 +601,8 @@ static int unpack_nondirectories(int n, unsigned long mask,\n \t}\n \n \tif (o->merge)\n-\t\treturn call_unpack_fn(src, o);\n+\t\treturn call_unpack_fn((const struct cache_entry * const *)src,\n+\t\t\t\t      o);\n \n \tfor (i = 0; i < n; i++)\n \t\tif (src[i] && src[i] != o->df_conflict_entry)\n@@ -1574,7 +1576,8 @@ static void show_stage_entry(FILE *o,\n }\n #endif\n \n-int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o)\n+int threeway_merge(const struct cache_entry * const *stages,\n+\t\t   struct unpack_trees_options *o)\n {\n \tconst struct cache_entry *index;\n \tconst struct cache_entry *head;\n@@ -1746,7 +1749,8 @@ int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o)\n  * \"carry forward\" rule, please see <Documentation/git-read-tree.txt>.\n  *\n  */\n-int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n+int twoway_merge(const struct cache_entry * const *src,\n+\t\t struct unpack_trees_options *o)\n {\n \tconst struct cache_entry *current = src[0];\n \tconst struct cache_entry *oldtree = src[1];\n@@ -1812,8 +1816,8 @@ int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n  * Keep the index entries at stage0, collapse stage1 but make sure\n  * stage0 does not have anything there.\n  */\n-int bind_merge(struct cache_entry **src,\n-\t\tstruct unpack_trees_options *o)\n+int bind_merge(const struct cache_entry * const *src,\n+\t       struct unpack_trees_options *o)\n {\n \tconst struct cache_entry *old = src[0];\n \tconst struct cache_entry *a = src[1];\n@@ -1836,7 +1840,8 @@ int bind_merge(struct cache_entry **src,\n  * The rule is:\n  * - take the stat information from stage0, take the data from stage1\n  */\n-int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n+int oneway_merge(const struct cache_entry * const *src,\n+\t\t struct unpack_trees_options *o)\n {\n \tconst struct cache_entry *old = src[0];\n \tconst struct cache_entry *a = src[1];\ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex 5e432f5..36a73a6 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -8,7 +8,7 @@\n struct unpack_trees_options;\n struct exclude_list;\n \n-typedef int (*merge_fn_t)(struct cache_entry **src,\n+typedef int (*merge_fn_t)(const struct cache_entry * const *src,\n \t\tstruct unpack_trees_options *options);\n \n enum unpack_trees_error_types {\n@@ -77,9 +77,13 @@ struct unpack_trees_options {\n extern int unpack_trees(unsigned n, struct tree_desc *t,\n \t\tstruct unpack_trees_options *options);\n \n-int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o);\n-int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o);\n-int bind_merge(struct cache_entry **src, struct unpack_trees_options *o);\n-int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o);\n+int threeway_merge(const struct cache_entry * const *stages,\n+\t\t   struct unpack_trees_options *o);\n+int twoway_merge(const struct cache_entry * const *src,\n+\t\t struct unpack_trees_options *o);\n+int bind_merge(const struct cache_entry * const *src,\n+\t       struct unpack_trees_options *o);\n+int oneway_merge(const struct cache_entry * const *src,\n+\t\t struct unpack_trees_options *o);\n \n #endif\n-- \n1.8.3\n"},{"id":"218970","messageId":"1369913664-49734-8-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"33985","inReplyTo":"1369913664-49734-1-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 7/7] unpack-trees: free cache_entry array members for merges","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-30T11:34:24Z","receivedAt":"2013-05-30T11:34:24Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"The merge functions duplicate entries as needed and they don't free\nthem.  Release them in unpack_nondirectories, the same function\nwhere they were allocated, after we're done.\n\nSigned-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n unpack-trees.c | 11 ++++++++---\n 1 file changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 2dbc05d..fc0dd5a 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -600,9 +600,14 @@ static int unpack_nondirectories(int n, unsigned long mask,\n \t\tsrc[i + o->merge] = create_ce_entry(info, names + i, stage);\n \t}\n \n-\tif (o->merge)\n-\t\treturn call_unpack_fn((const struct cache_entry * const *)src,\n-\t\t\t\t      o);\n+\tif (o->merge) {\n+\t\tint rc = call_unpack_fn((const struct cache_entry * const *)src,\n+\t\t\t\t\to);\n+\t\tfor (i = 1; i <= n; i++)\n+\t\t\tif (src[i] && src[i] != o->df_conflict_entry)\n+\t\t\t\tfree(src[i]);\n+\t\treturn rc;\n+\t}\n \n \tfor (i = 0; i < n; i++)\n \t\tif (src[i] && src[i] != o->df_conflict_entry)\n-- \n1.8.3\n"},{"id":"218982","messageId":"CAMP44s2=YuBLHgz52ie-FZYU5iz4o4tY0-zH+6XuzpiupjomLA@mail.gmail.com","threadId":"33985","inReplyTo":"1369913664-49734-8-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"Re: [PATCH 7/7] unpack-trees: free cache_entry array members for merges","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-30T12:04:42Z","receivedAt":"2013-05-30T12:04:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 30, 2013 at 6:34 AM, René Scharfe\n<rene.scharfe@lsrfire.ath.cx> wrote:\n> The merge functions duplicate entries as needed and they don't free\n> them.  Release them in unpack_nondirectories, the same function\n> where they were allocated, after we're done.\n\nAh, you beat me to this change, but..\n\n> @@ -600,9 +600,14 @@ static int unpack_nondirectories(int n, unsigned long mask,\n>                 src[i + o->merge] = create_ce_entry(info, names + i, stage);\n>         }\n>\n> -       if (o->merge)\n> -               return call_unpack_fn((const struct cache_entry * const *)src,\n> -                                     o);\n> +       if (o->merge) {\n> +               int rc = call_unpack_fn((const struct cache_entry * const *)src,\n> +                                       o);\n> +               for (i = 1; i <= n; i++)\n> +                       if (src[i] && src[i] != o->df_conflict_entry)\n> +                               free(src[i]);\n\nDoesn't it make more sense to follow the code above and do src[i + o->merge]?\n\n-- \nFelipe Contreras\n"},{"id":"219013","messageId":"51A764CE.4000708@lsrfire.ath.cx","threadId":"33985","inReplyTo":"CAMP44s2=YuBLHgz52ie-FZYU5iz4o4tY0-zH+6XuzpiupjomLA@mail.gmail.com","subject":"Re: [PATCH 7/7] unpack-trees: free cache_entry array members for merges","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-30T14:40:14Z","receivedAt":"2013-05-30T14:40:14Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 30.05.2013 14:04, schrieb Felipe Contreras:\n> On Thu, May 30, 2013 at 6:34 AM, René Scharfe\n> <rene.scharfe@lsrfire.ath.cx> wrote:\n>> The merge functions duplicate entries as needed and they don't free\n>> them.  Release them in unpack_nondirectories, the same function\n>> where they were allocated, after we're done.\n>\n> Ah, you beat me to this change, but..\n>\n>> @@ -600,9 +600,14 @@ static int unpack_nondirectories(int n, unsigned long mask,\n>>                  src[i + o->merge] = create_ce_entry(info, names + i, stage);\n>>          }\n>>\n>> -       if (o->merge)\n>> -               return call_unpack_fn((const struct cache_entry * const *)src,\n>> -                                     o);\n>> +       if (o->merge) {\n>> +               int rc = call_unpack_fn((const struct cache_entry * const *)src,\n>> +                                       o);\n>> +               for (i = 1; i <= n; i++)\n>> +                       if (src[i] && src[i] != o->df_conflict_entry)\n>> +                               free(src[i]);\n>\n> Doesn't it make more sense to follow the code above and do src[i + o->merge]?\n\nNot sure I understand.  Is the goal to avoid confusion for code readers \nby using the same indexing method for allocation and release?  Or are \nyou worried about o->merge having a different value than 1 in that loop?\n\nWe'd have to add 1 (== o->merge) to each index variable usage with a \nzero-based loop.  A one-based loop avoids that, and while it's not \npretty it's also not too complicated, I think.\n\nRené\n"},{"id":"219021","messageId":"CAMP44s10a9fj4No6o3GZQrwkREQP2WXbXr+1F83Q11EJ1AxrcA@mail.gmail.com","threadId":"33985","inReplyTo":"51A764CE.4000708@lsrfire.ath.cx","subject":"Re: [PATCH 7/7] unpack-trees: free cache_entry array members for merges","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-30T15:20:48Z","receivedAt":"2013-05-30T15:20:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 30, 2013 at 9:40 AM, René Scharfe\n<rene.scharfe@lsrfire.ath.cx> wrote:\n> Am 30.05.2013 14:04, schrieb Felipe Contreras:\n>\n>> On Thu, May 30, 2013 at 6:34 AM, René Scharfe\n>> <rene.scharfe@lsrfire.ath.cx> wrote:\n>>>\n>>> The merge functions duplicate entries as needed and they don't free\n>>> them.  Release them in unpack_nondirectories, the same function\n>>> where they were allocated, after we're done.\n>>\n>>\n>> Ah, you beat me to this change, but..\n>>\n>>> @@ -600,9 +600,14 @@ static int unpack_nondirectories(int n, unsigned\n>>> long mask,\n>>>                  src[i + o->merge] = create_ce_entry(info, names + i,\n>>> stage);\n>>>          }\n>>>\n>>> -       if (o->merge)\n>>> -               return call_unpack_fn((const struct cache_entry * const\n>>> *)src,\n>>> -                                     o);\n>>> +       if (o->merge) {\n>>> +               int rc = call_unpack_fn((const struct cache_entry * const\n>>> *)src,\n>>> +                                       o);\n>>> +               for (i = 1; i <= n; i++)\n>>> +                       if (src[i] && src[i] != o->df_conflict_entry)\n>>> +                               free(src[i]);\n>>\n>>\n>> Doesn't it make more sense to follow the code above and do src[i +\n>> o->merge]?\n>\n>\n> Not sure I understand.  Is the goal to avoid confusion for code readers by\n> using the same indexing method for allocation and release?  Or are you\n> worried about o->merge having a different value than 1 in that loop?\n\nBoth. In particular I'm eyeballing the code you can even see in this patch:\n\n src[i + o->merge] = create_ce_entry(info, names + i, stage);\n\nIf you think it's better to use src[i], then I think the code above\nshould do the same.\n\n-- \nFelipe Contreras\n"},{"id":"219372","messageId":"alpine.DEB.2.00.1306041605260.1612@ds9.cixit.se","threadId":"33985","inReplyTo":"1369913664-49734-4-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"Re: [PATCH 3/7] unpack-trees: factor out dup_entry","fromName":"Peter Krefting","fromEmail":"peter@softwolves.pp.se","sentAt":"2013-06-04T15:06:08Z","receivedAt":"2013-06-04T15:06:08Z","isPatch":true,"sender":{"key":"peter@softwolves.pp.se","avatar":"https://avatars.githubusercontent.com/u/990764?v=4"},"body":"René Scharfe:\n\n> While we're add it,\n\n  \"add\" → \"at\"\n\n-- \n\\\\// Peter - http://www.softwolves.pp.se/\n"}]}