{"thread":{"id":"30949","subject":"[PATCH/RFC] Replace ce_namelen() with a ce_namelen field","startedAt":"2012-07-04T09:18:48Z","lastAt":"2012-07-11T16:35:15Z","messageCount":18,"participants":["Thomas Gummerer","Thomas Rast","Nguyen Thai Ngoc Duy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"194609","messageId":"1341393528-21037-1-git-send-email-t.gummerer@gmail.com","threadId":"30949","inReplyTo":null,"subject":"[PATCH/RFC] Replace ce_namelen() with a ce_namelen field","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2012-07-04T09:18:48Z","receivedAt":"2012-07-04T09:18:48Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Replace the ce_namelen() function in cache.h with a ce_namelen\nfield in struct cache_entry. This will both give us a tiny bit\nof a performance enhancement when working with long pathnames\nand is part of the refactoring for the index-v5 file format.\n\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n builtin/add.c            |    4 ++--\n builtin/apply.c          |    1 +\n builtin/blame.c          |    1 +\n builtin/checkout-index.c |    8 ++++----\n builtin/checkout.c       |    7 ++++---\n builtin/clean.c          |    2 +-\n builtin/commit.c         |    2 +-\n builtin/grep.c           |    2 +-\n builtin/ls-files.c       |    8 ++++----\n builtin/rm.c             |    2 +-\n builtin/update-index.c   |    9 ++++++---\n cache-tree.c             |    4 ++--\n cache.h                  |   11 ++---------\n diff-lib.c               |    6 +++---\n entry.c                  |    2 +-\n merge-recursive.c        |    2 +-\n name-hash.c              |    4 ++--\n preload-index.c          |    2 +-\n read-cache.c             |   43 ++++++++++++++++++++++++-------------------\n rerere.c                 |    2 +-\n resolve-undo.c           |    2 +-\n sha1_name.c              |    6 +++---\n submodule.c              |    2 +-\n tree.c                   |    1 +\n unpack-trees.c           |   31 ++++++++++++++++---------------\n 25 files changed, 85 insertions(+), 79 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex b79336d..63ef9c5 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -114,7 +114,7 @@ static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)\n \t\treturn;\n \tfor (i = 0; i < active_nr; i++) {\n \t\tstruct cache_entry *ce = active_cache[i];\n-\t\tmatch_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen);\n+\t\tmatch_pathspec(pathspec, ce->name, ce->ce_namelen, 0, seen);\n \t}\n }\n \n@@ -163,7 +163,7 @@ static void treat_gitlinks(const char **pathspec)\n \tfor (i = 0; i < active_nr; i++) {\n \t\tstruct cache_entry *ce = active_cache[i];\n \t\tif (S_ISGITLINK(ce->ce_mode)) {\n-\t\t\tint len = ce_namelen(ce), j;\n+\t\t\tint len = ce->ce_namelen, j;\n \t\t\tfor (j = 0; pathspec[j]; j++) {\n \t\t\t\tint len2 = strlen(pathspec[j]);\n \t\t\t\tif (len2 <= len || pathspec[j][len] != '/' ||\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex dda9ea0..c9fae60 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -3503,6 +3503,7 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n \tmemcpy(ce->name, path, namelen);\n \tce->ce_mode = create_ce_mode(mode);\n \tce->ce_flags = namelen;\n+\tce->ce_namelen = namelen;\n \tif (S_ISGITLINK(mode)) {\n \t\tconst char *s = buf;\n \ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 24d3dd5..72b2ed4 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2154,6 +2154,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \thashcpy(ce->sha1, origin->blob_sha1);\n \tmemcpy(ce->name, path, len);\n \tce->ce_flags = create_ce_flags(len, 0);\n+\tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\n \tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n \ndiff --git a/builtin/checkout-index.c b/builtin/checkout-index.c\nindex c16d82b..ecabe1a 100644\n--- a/builtin/checkout-index.c\n+++ b/builtin/checkout-index.c\n@@ -55,7 +55,7 @@ static int checkout_file(const char *name, int prefix_length)\n \n \twhile (pos < active_nr) {\n \t\tstruct cache_entry *ce = active_cache[pos];\n-\t\tif (ce_namelen(ce) != namelen ||\n+\t\tif (ce->ce_namelen != namelen ||\n \t\t    memcmp(ce->name, name, namelen))\n \t\t\tbreak;\n \t\thas_same_name = 1;\n@@ -100,12 +100,12 @@ static void checkout_all(const char *prefix, int prefix_length)\n \t\t    && (CHECKOUT_ALL != checkout_stage || !ce_stage(ce)))\n \t\t\tcontinue;\n \t\tif (prefix && *prefix &&\n-\t\t    (ce_namelen(ce) <= prefix_length ||\n+\t\t    (ce->ce_namelen <= prefix_length ||\n \t\t     memcmp(prefix, ce->name, prefix_length)))\n \t\t\tcontinue;\n \t\tif (last_ce && to_tempfile) {\n-\t\t\tif (ce_namelen(last_ce) != ce_namelen(ce)\n-\t\t\t    || memcmp(last_ce->name, ce->name, ce_namelen(ce)))\n+\t\t\tif (last_ce->ce_namelen != ce->ce_namelen\n+\t\t\t    || memcmp(last_ce->name, ce->name, ce->ce_namelen))\n \t\t\t\twrite_tempfile_record(last_ce->name, prefix_length);\n \t\t}\n \t\tif (checkout_entry(ce, &state,\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 3ddda34..e05f89c 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -74,6 +74,7 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,\n \tmemcpy(ce->name, base, baselen);\n \tmemcpy(ce->name + baselen, pathname, len - baselen);\n \tce->ce_flags = create_ce_flags(len, 0) | CE_UPDATE;\n+\tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\n \tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);\n \treturn 0;\n@@ -244,7 +245,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,\n \t\tstruct cache_entry *ce = active_cache[pos];\n \t\tif (source_tree && !(ce->ce_flags & CE_UPDATE))\n \t\t\tcontinue;\n-\t\tmatch_pathspec(pathspec, ce->name, ce_namelen(ce), 0, ps_matched);\n+\t\tmatch_pathspec(pathspec, ce->name, ce->ce_namelen, 0, ps_matched);\n \t}\n \n \tif (report_path_error(ps_matched, pathspec, prefix))\n@@ -257,7 +258,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,\n \t/* Any unmerged paths? */\n \tfor (pos = 0; pos < active_nr; pos++) {\n \t\tstruct cache_entry *ce = active_cache[pos];\n-\t\tif (match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL)) {\n+\t\tif (match_pathspec(pathspec, ce->name, ce->ce_namelen, 0, NULL)) {\n \t\t\tif (!ce_stage(ce))\n \t\t\t\tcontinue;\n \t\t\tif (opts->force) {\n@@ -284,7 +285,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,\n \t\tstruct cache_entry *ce = active_cache[pos];\n \t\tif (source_tree && !(ce->ce_flags & CE_UPDATE))\n \t\t\tcontinue;\n-\t\tif (match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL)) {\n+\t\tif (match_pathspec(pathspec, ce->name, ce->ce_namelen, 0, NULL)) {\n \t\t\tif (!ce_stage(ce)) {\n \t\t\t\terrs |= checkout_entry(ce, &state, NULL);\n \t\t\t\tcontinue;\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 0c7b3d0..512d31c 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -128,7 +128,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\tpos = -pos - 1;\n \t\tif (pos < active_nr) {\n \t\t\tce = active_cache[pos];\n-\t\t\tif (ce_namelen(ce) == len &&\n+\t\t\tif (ce->ce_namelen == len &&\n \t\t\t    !memcmp(ce->name, ent->name, len))\n \t\t\t\tcontinue; /* Yup, this one exists unmerged */\n \t\t}\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex a2ec73d..e08b87e 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -200,7 +200,7 @@ static int list_paths(struct string_list *list, const char *with_tree,\n \n \t\tif (ce->ce_flags & CE_UPDATE)\n \t\t\tcontinue;\n-\t\tif (!match_pathspec(pattern, ce->name, ce_namelen(ce), 0, m))\n+\t\tif (!match_pathspec(pattern, ce->name, ce->ce_namelen, 0, m))\n \t\t\tcontinue;\n \t\titem = string_list_insert(list, ce->name);\n \t\tif (ce_skip_worktree(ce))\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 643938d..dd3b1cc 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -416,7 +416,7 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int\n \t\tstruct cache_entry *ce = active_cache[nr];\n \t\tif (!S_ISREG(ce->ce_mode))\n \t\t\tcontinue;\n-\t\tif (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))\n+\t\tif (!match_pathspec_depth(pathspec, ce->name, ce->ce_namelen, 0, NULL))\n \t\t\tcontinue;\n \t\t/*\n \t\t * If CE_VALID is on, we assume worktree file and its cache entry\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 7cff175..64a3e6e 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -104,7 +104,7 @@ static void show_killed_files(struct dir_struct *dir)\n \t\t\t\t * ent->name in the cache.  Does it expect\n \t\t\t\t * ent->name to be a directory?\n \t\t\t\t */\n-\t\t\t\tlen = ce_namelen(active_cache[pos]);\n+\t\t\t\tlen = active_cache[pos]->ce_namelen;\n \t\t\t\tif ((ent->len < len) &&\n \t\t\t\t    !strncmp(active_cache[pos]->name,\n \t\t\t\t\t     ent->name, ent->len) &&\n@@ -130,10 +130,10 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)\n {\n \tint len = max_prefix_len;\n \n-\tif (len >= ce_namelen(ce))\n+\tif (len >= ce->ce_namelen)\n \t\tdie(\"git ls-files: internal error - cache entry not superset of prefix\");\n \n-\tif (!match_pathspec(pathspec, ce->name, ce_namelen(ce), len, ps_matched))\n+\tif (!match_pathspec(pathspec, ce->name, ce->ce_namelen, len, ps_matched))\n \t\treturn;\n \n \tif (tag && *tag && show_valid_bit &&\n@@ -162,7 +162,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)\n \t\t       find_unique_abbrev(ce->sha1,abbrev),\n \t\t       ce_stage(ce));\n \t}\n-\twrite_name(ce->name, ce_namelen(ce));\n+\twrite_name(ce->name, ce->ce_namelen);\n \tif (debug_mode) {\n \t\tprintf(\"  ctime: %d:%d\\n\", ce->ce_ctime.sec, ce->ce_ctime.nsec);\n \t\tprintf(\"  mtime: %d:%d\\n\", ce->ce_mtime.sec, ce->ce_mtime.nsec);\ndiff --git a/builtin/rm.c b/builtin/rm.c\nindex 90c8a50..9a636b7 100644\n--- a/builtin/rm.c\n+++ b/builtin/rm.c\n@@ -171,7 +171,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \n \tfor (i = 0; i < active_nr; i++) {\n \t\tstruct cache_entry *ce = active_cache[i];\n-\t\tif (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))\n+\t\tif (!match_pathspec(pathspec, ce->name, ce->ce_namelen, 0, seen))\n \t\t\tcontinue;\n \t\tALLOC_GROW(list.name, list.nr + 1, list.alloc);\n \t\tlist.name[list.nr++] = ce->name;\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 5f038d6..784b52b 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -96,6 +96,7 @@ static int add_one_path(struct cache_entry *old, const char *path, int len, stru\n \tce = xcalloc(1, size);\n \tmemcpy(ce->name, path, len);\n \tce->ce_flags = len;\n+\tce->ce_namelen = len;\n \tfill_stat_cache_info(ce, st);\n \tce->ce_mode = ce_mode_from_stat(old, st->st_mode);\n \n@@ -236,6 +237,7 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,\n \thashcpy(ce->sha1, sha1);\n \tmemcpy(ce->name, path, len);\n \tce->ce_flags = create_ce_flags(len, stage);\n+\tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\n \tif (assume_unchanged)\n \t\tce->ce_flags |= CE_VALID;\n@@ -434,6 +436,7 @@ static struct cache_entry *read_one_ent(const char *which,\n \thashcpy(ce->sha1, sha1);\n \tmemcpy(ce->name, path, namelen);\n \tce->ce_flags = create_ce_flags(namelen, stage);\n+\tce->ce_namelen = namelen;\n \tce->ce_mode = create_ce_mode(mode);\n \treturn ce;\n }\n@@ -453,7 +456,7 @@ static int unresolve_one(const char *path)\n \t\tif (pos < active_nr) {\n \t\t\tstruct cache_entry *ce = active_cache[pos];\n \t\t\tif (ce_stage(ce) &&\n-\t\t\t    ce_namelen(ce) == namelen &&\n+\t\t\t    ce->ce_namelen == namelen &&\n \t\t\t    !memcmp(ce->name, path, namelen))\n \t\t\t\treturn 0;\n \t\t}\n@@ -466,7 +469,7 @@ static int unresolve_one(const char *path)\n \t\tpos = -pos-1;\n \t\tif (pos < active_nr) {\n \t\t\tstruct cache_entry *ce = active_cache[pos];\n-\t\t\tif (ce_namelen(ce) == namelen &&\n+\t\t\tif (ce->ce_namelen == namelen &&\n \t\t\t    !memcmp(ce->name, path, namelen)) {\n \t\t\t\tfprintf(stderr,\n \t\t\t\t\t\"%s: skipping still unmerged path.\\n\",\n@@ -569,7 +572,7 @@ static int do_reupdate(int ac, const char **av,\n \t\t\tcontinue;\n \t\tif (has_head)\n \t\t\told = read_one_ent(NULL, head_sha1,\n-\t\t\t\t\t   ce->name, ce_namelen(ce), 0);\n+\t\t\t\t\t   ce->name, ce->ce_namelen, 0);\n \t\tif (old && ce->ce_mode == old->ce_mode &&\n \t\t    !hashcmp(ce->sha1, old->sha1)) {\n \t\t\tfree(old);\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 28ed657..b5f5cd0 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -270,7 +270,7 @@ static int update_one(struct cache_tree *it,\n \t\tint pathlen, sublen, subcnt;\n \n \t\tpath = ce->name;\n-\t\tpathlen = ce_namelen(ce);\n+\t\tpathlen = ce->ce_namelen;\n \t\tif (pathlen <= baselen || memcmp(base, path, baselen))\n \t\t\tbreak; /* at the end of this level */\n \n@@ -313,7 +313,7 @@ static int update_one(struct cache_tree *it,\n \t\tunsigned mode;\n \n \t\tpath = ce->name;\n-\t\tpathlen = ce_namelen(ce);\n+\t\tpathlen = ce->ce_namelen;\n \t\tif (pathlen <= baselen || memcmp(base, path, baselen))\n \t\t\tbreak; /* at the end of this level */\n \ndiff --git a/cache.h b/cache.h\nindex cc5048c..5f93f22 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -128,6 +128,7 @@ struct cache_entry {\n \tunsigned int ce_gid;\n \tunsigned int ce_size;\n \tunsigned int ce_flags;\n+\tunsigned int ce_namelen;\n \tunsigned char sha1[20];\n \tstruct cache_entry *next;\n \tstruct cache_entry *dir_next;\n@@ -205,15 +206,7 @@ static inline unsigned create_ce_flags(size_t len, unsigned stage)\n \treturn (len | (stage << CE_STAGESHIFT));\n }\n \n-static inline size_t ce_namelen(const struct cache_entry *ce)\n-{\n-\tsize_t len = ce->ce_flags & CE_NAMEMASK;\n-\tif (len < CE_NAMEMASK)\n-\t\treturn len;\n-\treturn strlen(ce->name + CE_NAMEMASK) + CE_NAMEMASK;\n-}\n-\n-#define ce_size(ce) cache_entry_size(ce_namelen(ce))\n+#define ce_size(ce) cache_entry_size(ce->ce_namelen)\n #define ce_stage(ce) ((CE_STAGEMASK & (ce)->ce_flags) >> CE_STAGESHIFT)\n #define ce_uptodate(ce) ((ce)->ce_flags & CE_UPTODATE)\n #define ce_skip_worktree(ce) ((ce)->ce_flags & CE_SKIP_WORKTREE)\ndiff --git a/diff-lib.c b/diff-lib.c\nindex fc0dff3..720391b 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -32,7 +32,7 @@ static int check_removed(const struct cache_entry *ce, struct stat *st)\n \t\t\treturn -1;\n \t\treturn 1;\n \t}\n-\tif (has_symlink_leading_path(ce->name, ce_namelen(ce)))\n+\tif (has_symlink_leading_path(ce->name, ce->ce_namelen))\n \t\treturn 1;\n \tif (S_ISDIR(st->st_mode)) {\n \t\tunsigned char sub[20];\n@@ -115,7 +115,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\tint num_compare_stages = 0;\n \t\t\tsize_t path_len;\n \n-\t\t\tpath_len = ce_namelen(ce);\n+\t\t\tpath_len = ce->ce_namelen;\n \n \t\t\tdpath = xmalloc(combine_diff_path_size(5, path_len));\n \t\t\tdpath->path = (char *) &(dpath->parent[5]);\n@@ -319,7 +319,7 @@ static int show_modified(struct rev_info *revs,\n \tif (revs->combine_merges && !cached &&\n \t    (hashcmp(sha1, old->sha1) || hashcmp(old->sha1, new->sha1))) {\n \t\tstruct combine_diff_path *p;\n-\t\tint pathlen = ce_namelen(new);\n+\t\tint pathlen = new->ce_namelen;\n \n \t\tp = xmalloc(combine_diff_path_size(2, pathlen));\n \t\tp->path = (char *) &p->parent[2];\ndiff --git a/entry.c b/entry.c\nindex 17a6bcc..57ee28a 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -242,7 +242,7 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t\n \n \tmemcpy(path, state->base_dir, len);\n \tstrcpy(path + len, ce->name);\n-\tlen += ce_namelen(ce);\n+\tlen += ce->ce_namelen;\n \n \tif (!check_path(path, len, &st, state->base_dir_len)) {\n \t\tunsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 680937c..9beed5d 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -254,7 +254,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)\n \t\t\tstruct cache_entry *ce = active_cache[i];\n \t\t\tif (ce_stage(ce))\n \t\t\t\tfprintf(stderr, \"BUG: %d %.*s\\n\", ce_stage(ce),\n-\t\t\t\t\t(int)ce_namelen(ce), ce->name);\n+\t\t\t\t\t(int)ce->ce_namelen, ce->name);\n \t\t}\n \t\tdie(\"Bug in merge-recursive.c\");\n \t}\ndiff --git a/name-hash.c b/name-hash.c\nindex d8d25c2..5f0c998 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -75,7 +75,7 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)\n \t\treturn;\n \tce->ce_flags |= CE_HASHED;\n \tce->next = ce->dir_next = NULL;\n-\thash = hash_name(ce->name, ce_namelen(ce));\n+\thash = hash_name(ce->name, ce->ce_namelen);\n \tpos = insert_hash(hash, ce, &istate->name_hash);\n \tif (pos) {\n \t\tce->next = *pos;\n@@ -125,7 +125,7 @@ static int slow_same_name(const char *name1, int len1, const char *name2, int le\n \n static int same_name(const struct cache_entry *ce, const char *name, int namelen, int icase)\n {\n-\tint len = ce_namelen(ce);\n+\tint len = ce->ce_namelen;\n \n \t/*\n \t * Always do exact compare, even if we want a case-ignoring comparison;\ndiff --git a/preload-index.c b/preload-index.c\nindex 49cb08d..95ca421 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -55,7 +55,7 @@ static void *preload_thread(void *_data)\n \t\t\tcontinue;\n \t\tif (!ce_path_match(ce, &pathspec))\n \t\t\tcontinue;\n-\t\tif (threaded_has_symlink_leading_path(&cache, ce->name, ce_namelen(ce)))\n+\t\tif (threaded_has_symlink_leading_path(&cache, ce->name, ce->ce_namelen))\n \t\t\tcontinue;\n \t\tif (lstat(ce->name, &st))\n \t\t\tcontinue;\ndiff --git a/read-cache.c b/read-cache.c\nindex ef355cc..01281ab 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -56,6 +56,7 @@ void rename_index_entry_at(struct index_state *istate, int nr, const char *new_n\n \tcopy_cache_entry(new, old);\n \tnew->ce_flags &= ~(CE_STATE_MASK | CE_NAMEMASK);\n \tnew->ce_flags |= (namelen >= CE_NAMEMASK ? CE_NAMEMASK : namelen);\n+\tnew->ce_namelen = namelen;\n \tmemcpy(new->name, new_name, namelen + 1);\n \n \tcache_tree_invalidate_path(istate->cache_tree, old->name);\n@@ -492,7 +493,7 @@ int remove_file_from_index(struct index_state *istate, const char *path)\n \n static int compare_name(struct cache_entry *ce, const char *path, int namelen)\n {\n-\treturn namelen != ce_namelen(ce) || memcmp(path, ce->name, namelen);\n+\treturn namelen != ce->ce_namelen || memcmp(path, ce->name, namelen);\n }\n \n static int index_name_pos_also_unmerged(struct index_state *istate,\n@@ -520,8 +521,8 @@ static int index_name_pos_also_unmerged(struct index_state *istate,\n \n static int different_name(struct cache_entry *ce, struct cache_entry *alias)\n {\n-\tint len = ce_namelen(ce);\n-\treturn ce_namelen(alias) != len || memcmp(ce->name, alias->name, len);\n+\tint len = ce->ce_namelen;\n+\treturn alias->ce_namelen != len || memcmp(ce->name, alias->name, len);\n }\n \n /*\n@@ -542,7 +543,7 @@ static struct cache_entry *create_alias_ce(struct cache_entry *ce, struct cache_\n \t\tdie(\"Will not add file alias '%s' ('%s' already exists in index)\", ce->name, alias->name);\n \n \t/* Ok, create the new entry using the name of the existing alias */\n-\tlen = ce_namelen(alias);\n+\tlen = alias->ce_namelen;\n \tnew = xcalloc(1, cache_entry_size(len));\n \tmemcpy(new->name, alias->name, len);\n \tcopy_cache_entry(new, ce);\n@@ -582,6 +583,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \tce = xcalloc(1, size);\n \tmemcpy(ce->name, path, namelen);\n \tce->ce_flags = namelen;\n+\tce->ce_namelen = namelen;\n \tif (!intent_only)\n \t\tfill_stat_cache_info(ce, st);\n \telse\n@@ -623,7 +625,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \t\t}\n \t}\n \n-\talias = index_name_exists(istate, ce->name, ce_namelen(ce), ignore_case);\n+\talias = index_name_exists(istate, ce->name, ce->ce_namelen, ignore_case);\n \tif (alias && !ce_stage(alias) && !ie_match_stat(istate, alias, st, ce_option)) {\n \t\t/* Nothing changed, really */\n \t\tfree(ce);\n@@ -684,6 +686,7 @@ struct cache_entry *make_cache_entry(unsigned int mode,\n \thashcpy(ce->sha1, sha1);\n \tmemcpy(ce->name, path, len);\n \tce->ce_flags = create_ce_flags(len, stage);\n+\tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\n \n \tif (refresh)\n@@ -694,13 +697,13 @@ struct cache_entry *make_cache_entry(unsigned int mode,\n \n int ce_same_name(struct cache_entry *a, struct cache_entry *b)\n {\n-\tint len = ce_namelen(a);\n-\treturn ce_namelen(b) == len && !memcmp(a->name, b->name, len);\n+\tint len = a->ce_namelen;\n+\treturn b->ce_namelen == len && !memcmp(a->name, b->name, len);\n }\n \n int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec)\n {\n-\treturn match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL);\n+\treturn match_pathspec_depth(pathspec, ce->name, ce->ce_namelen, 0, NULL);\n }\n \n /*\n@@ -772,14 +775,14 @@ static int has_file_name(struct index_state *istate,\n \t\t\t const struct cache_entry *ce, int pos, int ok_to_replace)\n {\n \tint retval = 0;\n-\tint len = ce_namelen(ce);\n+\tint len = ce->ce_namelen;\n \tint stage = ce_stage(ce);\n \tconst char *name = ce->name;\n \n \twhile (pos < istate->cache_nr) {\n \t\tstruct cache_entry *p = istate->cache[pos++];\n \n-\t\tif (len >= ce_namelen(p))\n+\t\tif (len >= p->ce_namelen)\n \t\t\tbreak;\n \t\tif (memcmp(name, p->name, len))\n \t\t\tbreak;\n@@ -807,7 +810,7 @@ static int has_dir_name(struct index_state *istate,\n \tint retval = 0;\n \tint stage = ce_stage(ce);\n \tconst char *name = ce->name;\n-\tconst char *slash = name + ce_namelen(ce);\n+\tconst char *slash = name + ce->ce_namelen;\n \n \tfor (;;) {\n \t\tint len;\n@@ -848,7 +851,7 @@ static int has_dir_name(struct index_state *istate,\n \t\t */\n \t\twhile (pos < istate->cache_nr) {\n \t\t\tstruct cache_entry *p = istate->cache[pos];\n-\t\t\tif ((ce_namelen(p) <= len) ||\n+\t\t\tif ((p->ce_namelen <= len) ||\n \t\t\t    (p->name[len] != '/') ||\n \t\t\t    memcmp(p->name, name, len))\n \t\t\t\tbreak; /* not our subdirectory */\n@@ -1235,8 +1238,8 @@ struct ondisk_cache_entry_extended {\n #define ondisk_cache_entry_size(len) align_flex_name(ondisk_cache_entry,len)\n #define ondisk_cache_entry_extended_size(len) align_flex_name(ondisk_cache_entry_extended,len)\n #define ondisk_ce_size(ce) (((ce)->ce_flags & CE_EXTENDED) ? \\\n-\t\t\t    ondisk_cache_entry_extended_size(ce_namelen(ce)) : \\\n-\t\t\t    ondisk_cache_entry_size(ce_namelen(ce)))\n+\t\t\t    ondisk_cache_entry_extended_size(ce->ce_namelen) : \\\n+\t\t\t    ondisk_cache_entry_size(ce->ce_namelen))\n \n static int verify_hdr(struct cache_header *hdr, unsigned long size)\n {\n@@ -1320,6 +1323,7 @@ static struct cache_entry *cache_entry_from_ondisk(struct ondisk_cache_entry *on\n \tce->ce_gid   = ntoh_l(ondisk->gid);\n \tce->ce_size  = ntoh_l(ondisk->size);\n \tce->ce_flags = flags;\n+\tce->ce_namelen = len;\n \thashcpy(ce->sha1, ondisk->sha1);\n \tmemcpy(ce->name, name, len);\n \tce->name[len] = '\\0';\n@@ -1681,7 +1685,7 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce,\n \t\tsize = ondisk_ce_size(ce);\n \t\tondisk = xcalloc(1, size);\n \t\tname = copy_cache_entry_to_ondisk(ondisk, ce);\n-\t\tmemcpy(name, ce->name, ce_namelen(ce));\n+\t\tmemcpy(name, ce->name, ce->ce_namelen);\n \t} else {\n \t\tint common, to_remove, prefix_size;\n \t\tunsigned char to_remove_vi[16];\n@@ -1698,15 +1702,15 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce,\n \t\t\tsize = offsetof(struct ondisk_cache_entry_extended, name);\n \t\telse\n \t\t\tsize = offsetof(struct ondisk_cache_entry, name);\n-\t\tsize += prefix_size + (ce_namelen(ce) - common + 1);\n+\t\tsize += prefix_size + (ce->ce_namelen - common + 1);\n \n \t\tondisk = xcalloc(1, size);\n \t\tname = copy_cache_entry_to_ondisk(ondisk, ce);\n \t\tmemcpy(name, to_remove_vi, prefix_size);\n-\t\tmemcpy(name + prefix_size, ce->name + common, ce_namelen(ce) - common);\n+\t\tmemcpy(name + prefix_size, ce->name + common, ce->ce_namelen - common);\n \n \t\tstrbuf_splice(previous_name, common, to_remove,\n-\t\t\t      ce->name + common, ce_namelen(ce) - common);\n+\t\t\t      ce->name + common, ce->ce_namelen - common);\n \t}\n \n \tresult = ce_write(c, fd, ondisk, size);\n@@ -1846,6 +1850,7 @@ int read_index_unmerged(struct index_state *istate)\n \t\tnew_ce = xcalloc(1, size);\n \t\tmemcpy(new_ce->name, ce->name, len);\n \t\tnew_ce->ce_flags = create_ce_flags(len, 0) | CE_CONFLICTED;\n+\t\tnew_ce->ce_namelen = len;\n \t\tnew_ce->ce_mode = ce->ce_mode;\n \t\tif (add_index_entry(istate, new_ce, 0))\n \t\t\treturn error(\"%s: cannot drop to stage #0\",\n@@ -1876,7 +1881,7 @@ int index_name_is_other(const struct index_state *istate, const char *name,\n \tpos = -pos - 1;\n \tif (pos < istate->cache_nr) {\n \t\tstruct cache_entry *ce = istate->cache[pos];\n-\t\tif (ce_namelen(ce) == namelen &&\n+\t\tif (ce->ce_namelen == namelen &&\n \t\t    !memcmp(ce->name, name, namelen))\n \t\t\treturn 0; /* Yup, this one exists unmerged */\n \t}\ndiff --git a/rerere.c b/rerere.c\nindex dcb525a..044190d 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -320,7 +320,7 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \t\tif (active_nr <= pos)\n \t\t\tbreak;\n \t\tce = active_cache[pos++];\n-\t\tif (ce_namelen(ce) != len || memcmp(ce->name, path, len)\n+\t\tif (ce->ce_namelen != len || memcmp(ce->name, path, len)\n \t\t    || ce_stage(ce) != i + 1)\n \t\t\tbreak;\n \t\tmmfile[i].ptr = read_sha1_file(ce->sha1, &type, &size);\ndiff --git a/resolve-undo.c b/resolve-undo.c\nindex 72b4612..6fdee85 100644\n--- a/resolve-undo.c\n+++ b/resolve-undo.c\n@@ -165,7 +165,7 @@ void unmerge_index(struct index_state *istate, const char **pathspec)\n \n \tfor (i = 0; i < istate->cache_nr; i++) {\n \t\tstruct cache_entry *ce = istate->cache[i];\n-\t\tif (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL))\n+\t\tif (!match_pathspec(pathspec, ce->name, ce->ce_namelen, 0, NULL))\n \t\t\tcontinue;\n \t\ti = unmerge_index_entry_at(istate, i);\n \t}\ndiff --git a/sha1_name.c b/sha1_name.c\nindex c633113..2c4af98 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -967,7 +967,7 @@ static void diagnose_invalid_index_path(int stage,\n \t\tpos = -pos - 1;\n \tif (pos < active_nr) {\n \t\tce = active_cache[pos];\n-\t\tif (ce_namelen(ce) == namelen &&\n+\t\tif (ce->ce_namelen == namelen &&\n \t\t    !memcmp(ce->name, filename, namelen))\n \t\t\tdie(\"Path '%s' is in the index, but not at stage %d.\\n\"\n \t\t\t    \"Did you mean ':%d:%s'?\",\n@@ -985,7 +985,7 @@ static void diagnose_invalid_index_path(int stage,\n \t\tpos = -pos - 1;\n \tif (pos < active_nr) {\n \t\tce = active_cache[pos];\n-\t\tif (ce_namelen(ce) == fullnamelen &&\n+\t\tif (ce->ce_namelen == fullnamelen &&\n \t\t    !memcmp(ce->name, fullname, fullnamelen))\n \t\t\tdie(\"Path '%s' is in the index, but not '%s'.\\n\"\n \t\t\t    \"Did you mean ':%d:%s' aka ':%d:./%s'?\",\n@@ -1087,7 +1087,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,\n \t\t\tpos = -pos - 1;\n \t\twhile (pos < active_nr) {\n \t\t\tce = active_cache[pos];\n-\t\t\tif (ce_namelen(ce) != namelen ||\n+\t\t\tif (ce->ce_namelen != namelen ||\n \t\t\t    memcmp(ce->name, cp, namelen))\n \t\t\t\tbreak;\n \t\t\tif (ce_stage(ce) == stage) {\ndiff --git a/submodule.c b/submodule.c\nindex 959d349..c5f5f15 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -112,7 +112,7 @@ void gitmodules_config(void)\n \t\t\tpos = -1 - pos;\n \t\t\tif (active_nr > pos) {  /* there is a .gitmodules */\n \t\t\t\tconst struct cache_entry *ce = active_cache[pos];\n-\t\t\t\tif (ce_namelen(ce) == 11 &&\n+\t\t\t\tif (ce->ce_namelen == 11 &&\n \t\t\t\t    !memcmp(ce->name, \".gitmodules\", 11))\n \t\t\t\t\tgitmodules_is_unmerged = 1;\n \t\t\t}\ndiff --git a/tree.c b/tree.c\nindex 676e9f7..f2a66be 100644\n--- a/tree.c\n+++ b/tree.c\n@@ -23,6 +23,7 @@ static int read_one_entry_opt(const unsigned char *sha1, const char *base, int b\n \n \tce->ce_mode = create_ce_mode(mode);\n \tce->ce_flags = create_ce_flags(baselen + len, stage);\n+\tce->ce_namelen = baselen + len;\n \tmemcpy(ce->name, base, baselen);\n \tmemcpy(ce->name + baselen, pathname, len+1);\n \thashcpy(ce->sha1, sha1);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex ad40109..e5e6fc5 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -175,11 +175,11 @@ static void display_error_msgs(struct unpack_trees_options *o)\n  */\n static void unlink_entry(struct cache_entry *ce)\n {\n-\tif (!check_leading_path(ce->name, ce_namelen(ce)))\n+\tif (!check_leading_path(ce->name, ce->ce_namelen))\n \t\treturn;\n \tif (remove_or_warn(ce->ce_mode, ce->name))\n \t\treturn;\n-\tschedule_dir_for_removal(ce->name, ce_namelen(ce));\n+\tschedule_dir_for_removal(ce->name, ce->ce_namelen);\n }\n \n static struct checkout state;\n@@ -324,7 +324,7 @@ static int locate_in_src_index(struct cache_entry *ce,\n \t\t\t       struct unpack_trees_options *o)\n {\n \tstruct index_state *index = o->src_index;\n-\tint len = ce_namelen(ce);\n+\tint len = ce->ce_namelen;\n \tint pos = index_name_pos(index, ce->name, len);\n \tif (pos < 0)\n \t\tpos = -1 - pos;\n@@ -340,12 +340,12 @@ static void mark_ce_used_same_name(struct cache_entry *ce,\n \t\t\t\t   struct unpack_trees_options *o)\n {\n \tstruct index_state *index = o->src_index;\n-\tint len = ce_namelen(ce);\n+\tint len = ce->ce_namelen;\n \tint pos;\n \n \tfor (pos = locate_in_src_index(ce, o); pos < index->cache_nr; pos++) {\n \t\tstruct cache_entry *next = index->cache[pos];\n-\t\tif (len != ce_namelen(next) ||\n+\t\tif (len != next->ce_namelen ||\n \t\t    memcmp(ce->name, next->name, len))\n \t\t\tbreak;\n \t\tmark_ce_used(next, o);\n@@ -370,14 +370,14 @@ static void add_same_unmerged(struct cache_entry *ce,\n \t\t\t      struct unpack_trees_options *o)\n {\n \tstruct index_state *index = o->src_index;\n-\tint len = ce_namelen(ce);\n+\tint len = ce->ce_namelen;\n \tint pos = index_name_pos(index, ce->name, len);\n \n \tif (0 <= pos)\n \t\tdie(\"programming error in a caller of mark_ce_used_same_name\");\n \tfor (pos = -pos - 1; pos < index->cache_nr; pos++) {\n \t\tstruct cache_entry *next = index->cache[pos];\n-\t\tif (len != ce_namelen(next) ||\n+\t\tif (len != next->ce_namelen ||\n \t\t    memcmp(ce->name, next->name, len))\n \t\t\tbreak;\n \t\tadd_entry(o, next, 0, 0);\n@@ -493,7 +493,7 @@ static int do_compare_entry(const struct cache_entry *ce, const struct traverse_\n \t\t\treturn cmp;\n \t}\n \tpathlen = info->pathlen;\n-\tce_len = ce_namelen(ce);\n+\tce_len = ce->ce_namelen;\n \n \t/* If ce_len < pathlen then we must have previously hit \"name == directory\" entry */\n \tif (ce_len < pathlen)\n@@ -516,7 +516,7 @@ static int compare_entry(const struct cache_entry *ce, const struct traverse_inf\n \t * Even if the beginning compared identically, the ce should\n \t * compare as bigger than a directory leading up to it!\n \t */\n-\treturn ce_namelen(ce) > traverse_path_len(info, n);\n+\treturn ce->ce_namelen > traverse_path_len(info, n);\n }\n \n static int ce_in_traverse_path(const struct cache_entry *ce,\n@@ -530,7 +530,7 @@ static int ce_in_traverse_path(const struct cache_entry *ce,\n \t * If ce (blob) is the same name as the path (which is a tree\n \t * we will be descending into), it won't be inside it.\n \t */\n-\treturn (info->pathlen < ce_namelen(ce));\n+\treturn (info->pathlen < ce->ce_namelen);\n }\n \n static struct cache_entry *create_ce_entry(const struct traverse_info *info, const struct name_entry *n, int stage)\n@@ -540,6 +540,7 @@ static struct cache_entry *create_ce_entry(const struct traverse_info *info, con\n \n \tce->ce_mode = create_ce_mode(n->mode);\n \tce->ce_flags = create_ce_flags(len, stage);\n+\tce->ce_namelen = len;\n \thashcpy(ce->sha1, n->sha1);\n \tmake_traverse_path(ce->name, info, n);\n \n@@ -657,7 +658,7 @@ static int find_cache_pos(struct traverse_info *info,\n \t\tif (ce_slash)\n \t\t\tce_len = ce_slash - ce_name;\n \t\telse\n-\t\t\tce_len = ce_namelen(ce) - pfxlen;\n+\t\t\tce_len = ce->ce_namelen - pfxlen;\n \t\tcmp = name_compare(p->path, p_len, ce_name, ce_len);\n \t\t/*\n \t\t * Exact match; if we have a directory we need to\n@@ -938,7 +939,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n \n \t\t/* Non-directory */\n \t\tdtype = ce_to_dtype(ce);\n-\t\tret = excluded_from_list(ce->name, ce_namelen(ce), name, &dtype, el);\n+\t\tret = excluded_from_list(ce->name, ce->ce_namelen, name, &dtype, el);\n \t\tif (ret < 0)\n \t\t\tret = defval;\n \t\tif (ret > 0)\n@@ -1293,7 +1294,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce,\n \t     i < o->src_index->cache_nr;\n \t     i++) {\n \t\tstruct cache_entry *ce2 = o->src_index->cache[i];\n-\t\tint len = ce_namelen(ce2);\n+\t\tint len = ce2->ce_namelen;\n \t\tif (len < namelen ||\n \t\t    strncmp(ce->name, ce2->name, namelen) ||\n \t\t    ce2->name[namelen] != '/')\n@@ -1411,7 +1412,7 @@ static int verify_absent_1(struct cache_entry *ce,\n \tif (o->index_only || o->reset || !o->update)\n \t\treturn 0;\n \n-\tlen = check_leading_path(ce->name, ce_namelen(ce));\n+\tlen = check_leading_path(ce->name, ce->ce_namelen);\n \tif (!len)\n \t\treturn 0;\n \telse if (len > 0) {\n@@ -1430,7 +1431,7 @@ static int verify_absent_1(struct cache_entry *ce,\n \t\t\t\t     strerror(errno));\n \t\treturn 0;\n \t} else {\n-\t\treturn check_ok_to_remove(ce->name, ce_namelen(ce),\n+\t\treturn check_ok_to_remove(ce->name, ce->ce_namelen,\n \t\t\t\t\t  ce_to_dtype(ce), ce, &st,\n \t\t\t\t\t  error_type, o);\n \t}\n-- \n1.7.10.GIT\n"},{"id":"194611","messageId":"87txxn3kxb.fsf@thomas.inf.ethz.ch","threadId":"30949","inReplyTo":"1341393528-21037-1-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH/RFC] Replace ce_namelen() with a ce_namelen field","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-07-04T10:00:48Z","receivedAt":"2012-07-04T10:00:48Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Thomas Gummerer <t.gummerer@gmail.com> writes:\n\n> Replace the ce_namelen() function in cache.h with a ce_namelen\n> field in struct cache_entry. This will both give us a tiny bit\n> of a performance enhancement when working with long pathnames\n> and is part of the refactoring for the index-v5 file format.\n\nExpand this, at least proportionally to the damage incurred.\n\nFor example, I know that you know that there's a good reason why we want\nto put it in a new field when reading from v5 :-)\n\n> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n> ---\n>  builtin/add.c            |    4 ++--\n>  builtin/apply.c          |    1 +\n>  builtin/blame.c          |    1 +\n>  builtin/checkout-index.c |    8 ++++----\n>  builtin/checkout.c       |    7 ++++---\n>  builtin/clean.c          |    2 +-\n>  builtin/commit.c         |    2 +-\n>  builtin/grep.c           |    2 +-\n>  builtin/ls-files.c       |    8 ++++----\n>  builtin/rm.c             |    2 +-\n>  builtin/update-index.c   |    9 ++++++---\n>  cache-tree.c             |    4 ++--\n>  cache.h                  |   11 ++---------\n>  diff-lib.c               |    6 +++---\n>  entry.c                  |    2 +-\n>  merge-recursive.c        |    2 +-\n>  name-hash.c              |    4 ++--\n>  preload-index.c          |    2 +-\n>  read-cache.c             |   43 ++++++++++++++++++++++++-------------------\n>  rerere.c                 |    2 +-\n>  resolve-undo.c           |    2 +-\n>  sha1_name.c              |    6 +++---\n>  submodule.c              |    2 +-\n>  tree.c                   |    1 +\n>  unpack-trees.c           |   31 ++++++++++++++++---------------\n>  25 files changed, 85 insertions(+), 79 deletions(-)\n[...]\n> diff --git a/cache.h b/cache.h\n> index cc5048c..5f93f22 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -128,6 +128,7 @@ struct cache_entry {\n>  \tunsigned int ce_gid;\n>  \tunsigned int ce_size;\n>  \tunsigned int ce_flags;\n> +\tunsigned int ce_namelen;\n>  \tunsigned char sha1[20];\n>  \tstruct cache_entry *next;\n>  \tstruct cache_entry *dir_next;\n> @@ -205,15 +206,7 @@ static inline unsigned create_ce_flags(size_t len, unsigned stage)\n>  \treturn (len | (stage << CE_STAGESHIFT));\n>  }\n>  \n> -static inline size_t ce_namelen(const struct cache_entry *ce)\n> -{\n> -\tsize_t len = ce->ce_flags & CE_NAMEMASK;\n> -\tif (len < CE_NAMEMASK)\n> -\t\treturn len;\n> -\treturn strlen(ce->name + CE_NAMEMASK) + CE_NAMEMASK;\n> -}\n\nAFAICT almost all of the changes relate to the change from ce_namelen(ce)\nto ce->ce_namelen.  What's stopping you from defining\n\n#define ce_namelen(ce) ((ce)->ce_namelen)\n\nas a simple compatibility macro?\n\nI'm not sure whether such internal backwards-compatibility is desirable,\nbut it would certainly make the patch much more reviewable.  If Junio\nwould rather see the conversion, maybe do it in a separate patch that\nsimply substitutes the ce->ce_namelen for all the ce_namelen(ce)?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"194612","messageId":"CACsJy8BjfJa5VWusARkP4g5h_BTHywMiwzmtd5b9S4Cpf7g30Q@mail.gmail.com","threadId":"30949","inReplyTo":"1341393528-21037-1-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH/RFC] Replace ce_namelen() with a ce_namelen field","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-07-04T10:01:49Z","receivedAt":"2012-07-04T10:01:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jul 4, 2012 at 4:18 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:\n> Replace the ce_namelen() function in cache.h with a ce_namelen\n> field in struct cache_entry. This will both give us a tiny bit\n> of a performance enhancement when working with long pathnames\n> and is part of the refactoring for the index-v5 file format.\n\nCan we just wrap ce->namelen in ce_namelen() instead? Less changes\nthis way. We can do the replacement once v5 patches come.\n\nstatic inline int ce_namelen(struct cache_entry *ce)\n{\n   return ce->namelen;\n}\n\nsomething like that.\n-- \nDuy\n"},{"id":"194613","messageId":"CACsJy8AHqjgDx3PT39LVMeGp=qc3tEqNi9gjMok9V7RG1Bspew@mail.gmail.com","threadId":"30949","inReplyTo":"1341393528-21037-1-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH/RFC] Replace ce_namelen() with a ce_namelen field","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-07-04T10:07:42Z","receivedAt":"2012-07-04T10:07:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jul 4, 2012 at 4:18 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:\n> Replace the ce_namelen() function in cache.h with a ce_namelen\n> field in struct cache_entry. This will both give us a tiny bit\n> of a performance enhancement when working with long pathnames\n> and is part of the refactoring for the index-v5 file format.\n\nIn the name of performance maybe you want to replace strlen(ce->name)\nwith ce_namelen(ce) or ce->ce_namelen. git-grep says there are three\noccurrences, but you may want to relax the search a bit because it's\nnot always \"strlen(ce->name)\".\n-- \nDuy\n"},{"id":"194624","messageId":"7vtxxns4z4.fsf@alter.siamese.dyndns.org","threadId":"30949","inReplyTo":"1341393528-21037-1-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH/RFC] Replace ce_namelen() with a ce_namelen field","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-04T19:26:07Z","receivedAt":"2012-07-04T19:26:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer <t.gummerer@gmail.com> writes:\n\n> Replace the ce_namelen() function in cache.h with a ce_namelen\n> field in struct cache_entry.\n\nNAK, at least in this form that duplicates the same information in\nce_namelen (new field) and bits in ce_flags, making it easier for\nthem to go out of sync with new bugs (or misconversions in this\npatch, if there is one).\n\nAt least, keep the ce_namelen() inline used in the existing calling\nsite, as the whole point of it is that they do not have to care how\nthe name length is stored or computed.\n"},{"id":"194727","messageId":"1341590850-15653-1-git-send-email-t.gummerer@gmail.com","threadId":"30949","inReplyTo":"7vtxxns4z4.fsf@alter.siamese.dyndns.org","subject":"Introduction of a ce_namelen field","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2012-07-06T16:07:28Z","receivedAt":"2012-07-06T16:07:28Z","isPatch":false,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Thanks to the review of Junio, Duy and Thomas here is a reroll of the\npatches, where the name length is separated from the flags in the\nin-memory format and which includes a little bit of a performance\noptimization by using the ce_namelen field instead of strlen() in\na couple of places thanks to the suggestion of Duy.\n\n[PATCH/RFC v2 1/2] Strip namelen out of ce_flags into a ce_namelen\n[PATCH/RFC v2 2/2] Replace strlen(ce->name) with ce_namlen\n\nbuiltin/apply.c        |    3 ++-\nbuiltin/blame.c        |    3 ++-\nbuiltin/checkout.c     |    3 ++-\nbuiltin/update-index.c |    9 ++++++---\ncache.h                |   18 ++++++------------\nread-cache.c           |   58 +++++++++++++++++++++++++++++++++++-----------------------\ntree.c                 |    7 ++++---\nunpack-trees.c         |    5 +++--\n8 files changed, 60 insertions(+), 46 deletions(-)\n"},{"id":"194728","messageId":"1341590850-15653-2-git-send-email-t.gummerer@gmail.com","threadId":"30949","inReplyTo":"1341590850-15653-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH/RFC v2 1/2] Strip namelen out of ce_flags into a ce_namelen field","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2012-07-06T16:07:29Z","receivedAt":"2012-07-06T16:07:29Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Strip the name length from the ce_flags field and move it\ninto its own ce_namelen field in struct cache_entry. This\nwill both give us a tiny bit of a performance enhancement\nwhen working with long pathnames and is part of the\nrefactoring for the index-v5 file format.\n\nIndex-v5 won't store the name length in the on disk index\nfile, so storing it in the flags wouldn't make sense for\nindex-v5.\n\nIt also enhances readability, by making it more clear what\nis a flag, and where the length is stored and make it clear\nwhich functions use stages in comparisions and which only\nuse the length.\n\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n builtin/apply.c        |    3 ++-\n builtin/blame.c        |    3 ++-\n builtin/checkout.c     |    3 ++-\n builtin/update-index.c |    9 +++++---\n cache.h                |   18 ++++++----------\n read-cache.c           |   54 +++++++++++++++++++++++++++++-------------------\n tree.c                 |    7 ++++---\n unpack-trees.c         |    3 ++-\n 8 files changed, 57 insertions(+), 43 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex dda9ea0..10f83fc 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -3502,7 +3502,8 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n \tce = xcalloc(1, ce_size);\n \tmemcpy(ce->name, path, namelen);\n \tce->ce_mode = create_ce_mode(mode);\n-\tce->ce_flags = namelen;\n+\tce->ce_flags = 0;\n+\tce->ce_namelen = namelen;\n \tif (S_ISGITLINK(mode)) {\n \t\tconst char *s = buf;\n \ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 24d3dd5..e181368 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2153,7 +2153,8 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \tce = xcalloc(1, size);\n \thashcpy(ce->sha1, origin->blob_sha1);\n \tmemcpy(ce->name, path, len);\n-\tce->ce_flags = create_ce_flags(len, 0);\n+\tce->ce_flags = 0;\n+\tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\n \tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n \ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 3ddda34..5c06444 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -73,7 +73,8 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,\n \thashcpy(ce->sha1, sha1);\n \tmemcpy(ce->name, base, baselen);\n \tmemcpy(ce->name + baselen, pathname, len - baselen);\n-\tce->ce_flags = create_ce_flags(len, 0) | CE_UPDATE;\n+\tce->ce_flags = CE_UPDATE;\n+\tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\n \tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);\n \treturn 0;\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 5f038d6..911090f 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -95,7 +95,8 @@ static int add_one_path(struct cache_entry *old, const char *path, int len, stru\n \tsize = cache_entry_size(len);\n \tce = xcalloc(1, size);\n \tmemcpy(ce->name, path, len);\n-\tce->ce_flags = len;\n+\tce->ce_flags = 0;\n+\tce->ce_namelen = len;\n \tfill_stat_cache_info(ce, st);\n \tce->ce_mode = ce_mode_from_stat(old, st->st_mode);\n \n@@ -235,7 +236,8 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,\n \n \thashcpy(ce->sha1, sha1);\n \tmemcpy(ce->name, path, len);\n-\tce->ce_flags = create_ce_flags(len, stage);\n+\tce->ce_flags = create_ce_flags(stage);\n+\tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\n \tif (assume_unchanged)\n \t\tce->ce_flags |= CE_VALID;\n@@ -433,7 +435,8 @@ static struct cache_entry *read_one_ent(const char *which,\n \n \thashcpy(ce->sha1, sha1);\n \tmemcpy(ce->name, path, namelen);\n-\tce->ce_flags = create_ce_flags(namelen, stage);\n+\tce->ce_flags = create_ce_flags(stage);\n+\tce->ce_namelen = namelen;\n \tce->ce_mode = create_ce_mode(mode);\n \treturn ce;\n }\ndiff --git a/cache.h b/cache.h\nindex cc5048c..14160b7 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -128,6 +128,7 @@ struct cache_entry {\n \tunsigned int ce_gid;\n \tunsigned int ce_size;\n \tunsigned int ce_flags;\n+\tunsigned int ce_namelen;\n \tunsigned char sha1[20];\n \tstruct cache_entry *next;\n \tstruct cache_entry *dir_next;\n@@ -198,21 +199,12 @@ static inline void copy_cache_entry(struct cache_entry *dst, struct cache_entry\n \tdst->ce_flags = (dst->ce_flags & ~CE_STATE_MASK) | state;\n }\n \n-static inline unsigned create_ce_flags(size_t len, unsigned stage)\n+static inline unsigned create_ce_flags(unsigned stage)\n {\n-\tif (len >= CE_NAMEMASK)\n-\t\tlen = CE_NAMEMASK;\n-\treturn (len | (stage << CE_STAGESHIFT));\n-}\n-\n-static inline size_t ce_namelen(const struct cache_entry *ce)\n-{\n-\tsize_t len = ce->ce_flags & CE_NAMEMASK;\n-\tif (len < CE_NAMEMASK)\n-\t\treturn len;\n-\treturn strlen(ce->name + CE_NAMEMASK) + CE_NAMEMASK;\n+\treturn (stage << CE_STAGESHIFT);\n }\n \n+#define ce_namelen(ce) ((ce)->ce_namelen)\n #define ce_size(ce) cache_entry_size(ce_namelen(ce))\n #define ce_stage(ce) ((CE_STAGEMASK & (ce)->ce_flags) >> CE_STAGESHIFT)\n #define ce_uptodate(ce) ((ce)->ce_flags & CE_UPTODATE)\n@@ -448,6 +440,7 @@ extern int discard_index(struct index_state *);\n extern int unmerged_index(const struct index_state *);\n extern int verify_path(const char *path);\n extern struct cache_entry *index_name_exists(struct index_state *istate, const char *name, int namelen, int igncase);\n+extern int index_name_stage_pos(const struct index_state *, const char *name, int stage, int namelen);\n extern int index_name_pos(const struct index_state *, const char *name, int namelen);\n #define ADD_CACHE_OK_TO_ADD 1\t\t/* Ok to add */\n #define ADD_CACHE_OK_TO_REPLACE 2\t/* Ok to replace file/directory */\n@@ -857,6 +850,7 @@ extern int validate_headref(const char *ref);\n extern int base_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);\n extern int df_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);\n extern int cache_name_compare(const char *name1, int len1, const char *name2, int len2);\n+extern int cache_name_stage_compare(const char *name1, int stage1, int len1, const char *name2, int stage2, int len2);\n \n extern void *read_object_with_reference(const unsigned char *sha1,\n \t\t\t\t\tconst char *required_type,\ndiff --git a/read-cache.c b/read-cache.c\nindex ef355cc..ea75c89 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -54,8 +54,8 @@ void rename_index_entry_at(struct index_state *istate, int nr, const char *new_n\n \n \tnew = xmalloc(cache_entry_size(namelen));\n \tcopy_cache_entry(new, old);\n-\tnew->ce_flags &= ~(CE_STATE_MASK | CE_NAMEMASK);\n-\tnew->ce_flags |= (namelen >= CE_NAMEMASK ? CE_NAMEMASK : namelen);\n+\tnew->ce_flags &= ~CE_STATE_MASK;\n+\tnew->ce_namelen = namelen;\n \tmemcpy(new->name, new_name, namelen + 1);\n \n \tcache_tree_invalidate_path(istate->cache_tree, old->name);\n@@ -395,10 +395,8 @@ int df_name_compare(const char *name1, int len1, int mode1,\n \treturn c1 - c2;\n }\n \n-int cache_name_compare(const char *name1, int flags1, const char *name2, int flags2)\n+int cache_name_stage_compare(const char *name1, int stage1, int len1, const char *name2, int stage2, int len2)\n {\n-\tint len1 = flags1 & CE_NAMEMASK;\n-\tint len2 = flags2 & CE_NAMEMASK;\n \tint len = len1 < len2 ? len1 : len2;\n \tint cmp;\n \n@@ -410,18 +408,19 @@ int cache_name_compare(const char *name1, int flags1, const char *name2, int fla\n \tif (len1 > len2)\n \t\treturn 1;\n \n-\t/* Compare stages  */\n-\tflags1 &= CE_STAGEMASK;\n-\tflags2 &= CE_STAGEMASK;\n-\n-\tif (flags1 < flags2)\n+\tif (stage1 < stage2)\n \t\treturn -1;\n-\tif (flags1 > flags2)\n+\tif (stage1 > stage2)\n \t\treturn 1;\n \treturn 0;\n }\n \n-int index_name_pos(const struct index_state *istate, const char *name, int namelen)\n+int cache_name_compare(const char *name1, int len1, const char *name2, int len2)\n+{\n+\treturn cache_name_stage_compare(name1, 0, len1, name2, 0, len2);\n+}\n+\n+int index_name_stage_pos(const struct index_state *istate, const char *name, int stage, int namelen)\n {\n \tint first, last;\n \n@@ -430,7 +429,7 @@ int index_name_pos(const struct index_state *istate, const char *name, int namel\n \twhile (last > first) {\n \t\tint next = (last + first) >> 1;\n \t\tstruct cache_entry *ce = istate->cache[next];\n-\t\tint cmp = cache_name_compare(name, namelen, ce->name, ce->ce_flags);\n+\t\tint cmp = cache_name_stage_compare(name, stage, namelen, ce->name, ce_stage(ce), ce->ce_namelen);\n \t\tif (!cmp)\n \t\t\treturn next;\n \t\tif (cmp < 0) {\n@@ -442,6 +441,11 @@ int index_name_pos(const struct index_state *istate, const char *name, int namel\n \treturn -first-1;\n }\n \n+int index_name_pos(const struct index_state *istate, const char *name, int namelen)\n+{\n+\treturn index_name_stage_pos(istate, name, 0, namelen);\n+}\n+\n /* Remove entry, return true if there are more entries to go.. */\n int remove_index_entry_at(struct index_state *istate, int pos)\n {\n@@ -581,7 +585,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \tsize = cache_entry_size(namelen);\n \tce = xcalloc(1, size);\n \tmemcpy(ce->name, path, namelen);\n-\tce->ce_flags = namelen;\n+\tce->ce_namelen = namelen;\n \tif (!intent_only)\n \t\tfill_stat_cache_info(ce, st);\n \telse\n@@ -683,7 +687,8 @@ struct cache_entry *make_cache_entry(unsigned int mode,\n \n \thashcpy(ce->sha1, sha1);\n \tmemcpy(ce->name, path, len);\n-\tce->ce_flags = create_ce_flags(len, stage);\n+\tce->ce_flags = create_ce_flags(stage);\n+\tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\n \n \tif (refresh)\n@@ -820,7 +825,7 @@ static int has_dir_name(struct index_state *istate,\n \t\t}\n \t\tlen = slash - name;\n \n-\t\tpos = index_name_pos(istate, name, create_ce_flags(len, stage));\n+\t\tpos = index_name_stage_pos(istate, name, stage, len);\n \t\tif (pos >= 0) {\n \t\t\t/*\n \t\t\t * Found one, but not so fast.  This could\n@@ -910,7 +915,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n \tint new_only = option & ADD_CACHE_NEW_ONLY;\n \n \tcache_tree_invalidate_path(istate->cache_tree, ce->name);\n-\tpos = index_name_pos(istate, ce->name, ce->ce_flags);\n+\tpos = index_name_stage_pos(istate, ce->name, ce_stage(ce), ce->ce_namelen);\n \n \t/* existing match? Just replace it. */\n \tif (pos >= 0) {\n@@ -942,7 +947,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n \t\tif (!ok_to_replace)\n \t\t\treturn error(\"'%s' appears as both a file and as a directory\",\n \t\t\t\t     ce->name);\n-\t\tpos = index_name_pos(istate, ce->name, ce->ce_flags);\n+\t\tpos = index_name_stage_pos(istate, ce->name, ce_stage(ce), ce->ce_namelen);\n \t\tpos = -pos-1;\n \t}\n \treturn pos + 1;\n@@ -1319,7 +1324,8 @@ static struct cache_entry *cache_entry_from_ondisk(struct ondisk_cache_entry *on\n \tce->ce_uid   = ntoh_l(ondisk->uid);\n \tce->ce_gid   = ntoh_l(ondisk->gid);\n \tce->ce_size  = ntoh_l(ondisk->size);\n-\tce->ce_flags = flags;\n+\tce->ce_flags = flags & ~CE_NAMEMASK;\n+\tce->ce_namelen = len;\n \thashcpy(ce->sha1, ondisk->sha1);\n \tmemcpy(ce->name, name, len);\n \tce->name[len] = '\\0';\n@@ -1743,7 +1749,7 @@ int write_index(struct index_state *istate, int newfd)\n {\n \tgit_SHA_CTX c;\n \tstruct cache_header hdr;\n-\tint i, err, removed, extended, hdr_version;\n+\tint i, err, removed, extended, hdr_version, len;\n \tstruct cache_entry **cache = istate->cache;\n \tint entries = istate->cache_nr;\n \tstruct stat st;\n@@ -1759,6 +1765,11 @@ int write_index(struct index_state *istate, int newfd)\n \t\t\textended++;\n \t\t\tcache[i]->ce_flags |= CE_EXTENDED;\n \t\t}\n+\t\tif (cache[i]->ce_namelen >= CE_NAMEMASK)\n+\t\t\tlen = CE_NAMEMASK;\n+\t\telse\n+\t\t\tlen = cache[i]->ce_namelen;\n+\t\tcache[i]->ce_flags |= len;\n \t}\n \n \tif (!istate->version)\n@@ -1845,7 +1856,8 @@ int read_index_unmerged(struct index_state *istate)\n \t\tsize = cache_entry_size(len);\n \t\tnew_ce = xcalloc(1, size);\n \t\tmemcpy(new_ce->name, ce->name, len);\n-\t\tnew_ce->ce_flags = create_ce_flags(len, 0) | CE_CONFLICTED;\n+\t\tnew_ce->ce_flags = CE_CONFLICTED;\n+\t\tnew_ce->ce_namelen = len;\n \t\tnew_ce->ce_mode = ce->ce_mode;\n \t\tif (add_index_entry(istate, new_ce, 0))\n \t\t\treturn error(\"%s: cannot drop to stage #0\",\ndiff --git a/tree.c b/tree.c\nindex 676e9f7..7ab02d8 100644\n--- a/tree.c\n+++ b/tree.c\n@@ -22,7 +22,8 @@ static int read_one_entry_opt(const unsigned char *sha1, const char *base, int b\n \tce = xcalloc(1, size);\n \n \tce->ce_mode = create_ce_mode(mode);\n-\tce->ce_flags = create_ce_flags(baselen + len, stage);\n+\tce->ce_flags = create_ce_flags(stage);\n+\tce->ce_namelen = baselen + len;\n \tmemcpy(ce->name, base, baselen);\n \tmemcpy(ce->name + baselen, pathname, len+1);\n \thashcpy(ce->sha1, sha1);\n@@ -133,8 +134,8 @@ static int cmp_cache_name_compare(const void *a_, const void *b_)\n \n \tce1 = *((const struct cache_entry **)a_);\n \tce2 = *((const struct cache_entry **)b_);\n-\treturn cache_name_compare(ce1->name, ce1->ce_flags,\n-\t\t\t\t  ce2->name, ce2->ce_flags);\n+\treturn cache_name_stage_compare(ce1->name, ce_stage(ce1), ce1->ce_namelen,\n+\t\t\t\t  ce2->name, ce_stage(ce2), ce2->ce_namelen);\n }\n \n int read_tree(struct tree *tree, int stage, struct pathspec *match)\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex ad40109..9981dd3 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -539,7 +539,8 @@ static struct cache_entry *create_ce_entry(const struct traverse_info *info, con\n \tstruct cache_entry *ce = xcalloc(1, cache_entry_size(len));\n \n \tce->ce_mode = create_ce_mode(n->mode);\n-\tce->ce_flags = create_ce_flags(len, stage);\n+\tce->ce_flags = create_ce_flags(stage);\n+\tce->ce_namelen = len;\n \thashcpy(ce->sha1, n->sha1);\n \tmake_traverse_path(ce->name, info, n);\n \n-- \n1.7.10.GIT\n"},{"id":"194726","messageId":"1341590850-15653-3-git-send-email-t.gummerer@gmail.com","threadId":"30949","inReplyTo":"1341590850-15653-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH/RFC v2 2/2] Replace strlen() with ce_namelen()","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2012-07-06T16:07:30Z","receivedAt":"2012-07-06T16:07:30Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Replace strlen(ce->name) with ce_namelen() in a couple\nof places which gives us some additional bits of\nperformance.\n\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n read-cache.c   |    4 ++--\n unpack-trees.c |    2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex ea75c89..a77877a 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1124,7 +1124,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\t\tcontinue;\n \n \t\tif (pathspec &&\n-\t\t    !match_pathspec(pathspec, ce->name, strlen(ce->name), 0, seen))\n+\t\t    !match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))\n \t\t\tfiltered = 1;\n \n \t\tif (ce_stage(ce)) {\n@@ -1852,7 +1852,7 @@ int read_index_unmerged(struct index_state *istate)\n \t\tif (!ce_stage(ce))\n \t\t\tcontinue;\n \t\tunmerged = 1;\n-\t\tlen = strlen(ce->name);\n+\t\tlen = ce_namelen(ce);\n \t\tsize = cache_entry_size(len);\n \t\tnew_ce = xcalloc(1, size);\n \t\tmemcpy(new_ce->name, ce->name, len);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 9981dd3..abd0988 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1289,7 +1289,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce,\n \t * First let's make sure we do not have a local modification\n \t * in that directory.\n \t */\n-\tnamelen = strlen(ce->name);\n+\tnamelen = ce_namelen(ce);\n \tfor (i = locate_in_src_index(ce, o);\n \t     i < o->src_index->cache_nr;\n \t     i++) {\n-- \n1.7.10.GIT\n"},{"id":"194798","messageId":"7v4nphmyxc.fsf@alter.siamese.dyndns.org","threadId":"30949","inReplyTo":"1341590850-15653-3-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH/RFC v2 2/2] Replace strlen() with ce_namelen()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-09T02:49:19Z","receivedAt":"2012-07-09T02:49:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer <t.gummerer@gmail.com> writes:\n\n> Replace strlen(ce->name) with ce_namelen() in a couple\n> of places which gives us some additional bits of\n> performance.\n>\n> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n\nVery sensible, with or without the previous patch.\n\nI am kind of surprised that we are very good and have only these\nthree places that had these unnecessary pessimization.\n\n> ---\n>  read-cache.c   |    4 ++--\n>  unpack-trees.c |    2 +-\n>  2 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/read-cache.c b/read-cache.c\n> index ea75c89..a77877a 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -1124,7 +1124,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n>  \t\t\tcontinue;\n>  \n>  \t\tif (pathspec &&\n> -\t\t    !match_pathspec(pathspec, ce->name, strlen(ce->name), 0, seen))\n> +\t\t    !match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))\n>  \t\t\tfiltered = 1;\n>  \n>  \t\tif (ce_stage(ce)) {\n> @@ -1852,7 +1852,7 @@ int read_index_unmerged(struct index_state *istate)\n>  \t\tif (!ce_stage(ce))\n>  \t\t\tcontinue;\n>  \t\tunmerged = 1;\n> -\t\tlen = strlen(ce->name);\n> +\t\tlen = ce_namelen(ce);\n>  \t\tsize = cache_entry_size(len);\n>  \t\tnew_ce = xcalloc(1, size);\n>  \t\tmemcpy(new_ce->name, ce->name, len);\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 9981dd3..abd0988 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -1289,7 +1289,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce,\n>  \t * First let's make sure we do not have a local modification\n>  \t * in that directory.\n>  \t */\n> -\tnamelen = strlen(ce->name);\n> +\tnamelen = ce_namelen(ce);\n>  \tfor (i = locate_in_src_index(ce, o);\n>  \t     i < o->src_index->cache_nr;\n>  \t     i++) {\n"},{"id":"194799","messageId":"7vy5mtlebz.fsf@alter.siamese.dyndns.org","threadId":"30949","inReplyTo":"1341590850-15653-2-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH/RFC v2 1/2] Strip namelen out of ce_flags into a ce_namelen field","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-09T04:59:28Z","receivedAt":"2012-07-09T04:59:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer <t.gummerer@gmail.com> writes:\n\n> Strip the name length from the ce_flags field and move it\n> into its own ce_namelen field in struct cache_entry. This\n> will both give us a tiny bit of a performance enhancement\n> when working with long pathnames and is part of the\n> refactoring for the index-v5 file format.\n\nCareful.\n\nI do not mind moving name length out of low bits of ce_flags in\nprinciple, but if you claim this helps performance, you need to\nsubstanticate it.\n\nFYI, the current layout was designed for performance to take\nadvantage of the fact that most paths are short (among 39k paths in\nthe kernel tree, the longest seems to be 80 bytes long) and using\nstrlen() is rarely needed (the code even skips the first 4k bytes\nwhen it has to use strlen()).  It of course also shaves a few\nhundred kilobytes of memory necessary to hold the length separately\nin cache entries).\n\n> Index-v5 won't store the name length in the on disk index\n> file, so storing it in the flags wouldn't make sense for\n> index-v5.\n\nI think this split could make sense even without a new on-disk\nformat.\n\nOn the other hand, even if the result of this patch were to prove\nundesirable, it is still possible for the read_index code for v5\nformat to convert from its ondisk_cache_entry (which does not have\nlength anywhere) to the canonical cache_entry struct (which stores\nlength for common short path names in lower bits of flags field),\nand write_index code could do the reverse.\n\nBecause of the above two points, what index-v5 has or does not have\nis mostly immaterial when judging this patch, unless such a\nconversion has very high cost.\n\n> It also enhances readability, by making it more clear what\n> is a flag, and where the length is stored and make it clear\n> which functions use stages in comparisions and which only\n> use the length.\n\nThat can be true, but if we were to go this route, the patch should\nbe able to make CE_NAMEMASK totally private to the codepath that\nreads and writes v2/v3/v4 format and nowhere else.  The ce_flags\nfield in \"struct cache_entry\" that is visible to everybody from\ncache.h shouldn't need the padding of 12-low bits that is not used\n(I think CE_STAGESHIFT could even become 0, even though I do not see\nan immediate need for such a change).\n\nShouldn't \"struct ondisk_cache_entry\" in read-cache.c the only place\nthat need to know about this old layout, no?\n\n> diff --git a/builtin/apply.c b/builtin/apply.c\n> index dda9ea0..10f83fc 100644\n> --- a/builtin/apply.c\n> +++ b/builtin/apply.c\n> @@ -3502,7 +3502,8 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n>  \tce = xcalloc(1, ce_size);\n>  \tmemcpy(ce->name, path, namelen);\n>  \tce->ce_mode = create_ce_mode(mode);\n> -\tce->ce_flags = namelen;\n\nI think this should have been create_ce_flags(namelen, 0)\noriginally, so I would prefer the new code to spell it as\ncreate_ce_flags(0).\n\n> +\tce->ce_flags = 0;\n> +\tce->ce_namelen = namelen;\n\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index 24d3dd5..e181368 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -2153,7 +2153,8 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n>  \tce = xcalloc(1, size);\n>  \thashcpy(ce->sha1, origin->blob_sha1);\n>  \tmemcpy(ce->name, path, len);\n> -\tce->ce_flags = create_ce_flags(len, 0);\n> +\tce->ce_flags = 0;\n\nLikewise.  I think it is more in line with your \"making it more\nclear\" if you said create_ce_flags(0) here.  I won't repeat this for\nthe remainder of the patch where a bare number is stored without\nusing create_ce_flags(stage).\n\n> +#define ce_namelen(ce) ((ce)->ce_namelen)\n\nGood.\n\n> @@ -448,6 +440,7 @@ extern int discard_index(struct index_state *);\n>  extern int unmerged_index(const struct index_state *);\n>  extern int verify_path(const char *path);\n>  extern struct cache_entry *index_name_exists(struct index_state *istate, const char *name, int namelen, int igncase);\n> +extern int index_name_stage_pos(const struct index_state *, const char *name, int stage, int namelen);\n\nThe name string and its length form a pair; keep them next to each\nother (i.e. swap stage and namelen).\n\n> @@ -857,6 +850,7 @@ extern int validate_headref(const char *ref);\n>  extern int base_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);\n>  extern int df_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);\n>  extern int cache_name_compare(const char *name1, int len1, const char *name2, int len2);\n> +extern int cache_name_stage_compare(const char *name1, int stage1, int len1, const char *name2, int stage2, int len2);\n\nLikewise.\n\n> diff --git a/read-cache.c b/read-cache.c\n> index ef355cc..ea75c89 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -395,10 +395,8 @@ int df_name_compare(const char *name1, int len1, int mode1,\n>  \treturn c1 - c2;\n>  }\n>  \n> -int cache_name_compare(const char *name1, int flags1, const char *name2, int flags2)\n> +int cache_name_stage_compare(const char *name1, int stage1, int len1, const char *name2, int stage2, int len2)\n>  {\n> -\tint len1 = flags1 & CE_NAMEMASK;\n> -\tint len2 = flags2 & CE_NAMEMASK;\n>  \tint len = len1 < len2 ? len1 : len2;\n>  \tint cmp;\n\nIsn't this a _BUGFIX_?  It appears to me that the original code\nwould only compare the first 4k bytes and ignore the rest, if two\ncache entries, both with overlong names, are compared.  Care to come\nup with a test case to demonstrate the breakage and a bugfix without\nthe remainder of this patch, to be applied to 'master' and older\nmaintenance releases?  Perpahs something along the lines of this:\n\n read-cache.c | 14 +++++++++-----\n 1 file changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 1df6adf..d226e5e 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -397,11 +397,15 @@ int df_name_compare(const char *name1, int len1, int mode1,\n \n int cache_name_compare(const char *name1, int flags1, const char *name2, int flags2)\n {\n-\tint len1 = flags1 & CE_NAMEMASK;\n-\tint len2 = flags2 & CE_NAMEMASK;\n-\tint len = len1 < len2 ? len1 : len2;\n-\tint cmp;\n-\n+\tint len1, len2, len, cmp;\n+\n+\tlen1 = flags1 & CE_NAMEMASK;\n+\tif (len1 >= CE_NAMEMASK)\n+\t\tlen1 = strlen(name1)\n+\tlen2 = flags2 & CE_NAMEMASK;\n+\tif (len2 >= CE_NAMEMASK)\n+\t\tlen2 = strlen(name2);\n+\tlen = len1 < len2 ? len1 : len2;\n \tcmp = memcmp(name1, name2, len);\n \tif (cmp)\n \t\treturn cmp;\n\nPros and cons of adding a new ce_namelen field can be discussed\nseparately and built on top of such a bugfix.\n\n> @@ -1319,7 +1324,8 @@ static struct cache_entry *cache_entry_from_ondisk(struct ondisk_cache_entry *on\n>  \tce->ce_uid   = ntoh_l(ondisk->uid);\n>  \tce->ce_gid   = ntoh_l(ondisk->gid);\n>  \tce->ce_size  = ntoh_l(ondisk->size);\n> -\tce->ce_flags = flags;\n> +\tce->ce_flags = flags & ~CE_NAMEMASK;\n> +\tce->ce_namelen = len;\n\nOK, so this part wants to make (ce->ce_flags & CE_NAMEMASK) MBZ\n(must be zero), which is a step in the right direction (provided\nthat the creation of ce_namelen field is the right thing to do, that\nis).\n\n> @@ -1743,7 +1749,7 @@ int write_index(struct index_state *istate, int newfd)\n>  {\n>  \tgit_SHA_CTX c;\n>  \tstruct cache_header hdr;\n> -\tint i, err, removed, extended, hdr_version;\n> +\tint i, err, removed, extended, hdr_version, len;\n>  \tstruct cache_entry **cache = istate->cache;\n>  \tint entries = istate->cache_nr;\n>  \tstruct stat st;\n> @@ -1759,6 +1765,11 @@ int write_index(struct index_state *istate, int newfd)\n>  \t\t\textended++;\n>  \t\t\tcache[i]->ce_flags |= CE_EXTENDED;\n>  \t\t}\n> +\t\tif (cache[i]->ce_namelen >= CE_NAMEMASK)\n> +\t\t\tlen = CE_NAMEMASK;\n> +\t\telse\n> +\t\t\tlen = cache[i]->ce_namelen;\n> +\t\tcache[i]->ce_flags |= len;\n>  \t}\n\nThis is extremely dubious.  Didn't the earlier hunk at 1319 declare\nthat low bits of ce_flags is MBZ?  I would understand it if\nsomething like this is done to ondisk in ce_write_entry(), but I do\nnot think it is consistent to contaminate the low bits of ce_flags\nhere while your cache_entry_from_ondisk() clears them.\n"},{"id":"194896","messageId":"1341998558-8502-1-git-send-email-t.gummerer@gmail.com","threadId":"30949","inReplyTo":"7vy5mtlebz.fsf@alter.siamese.dyndns.org","subject":"[PATCH v3 0/3] Introduction of a ce_namelen field","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2012-07-11T09:22:35Z","receivedAt":"2012-07-11T09:22:35Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Thanks to Junio for reviewing v2 of this patch series.\n\nThe speedup coming from introducing the field exists, but is minimal,\nhere are the times for 1000 runs of git ls-files on the Webkit index,\nfirst without the ce_namelen field, and then with the ce_namelen field.\n$ time ./test.sh\n\nreal 4m40.895s\nuser 3m39.642s\nsys 0m40.159s\n\n$ time ./test.sh\n\nreal 4m34.872s\nuser 3m37.176s\nsys 0m40.401s\n\n1/3 is a bugfix for very long files which share at least 4096\ncharacters in their filename. It also includes a test, tough I'm\nnot sure I have put it in the right place.\n\nThe other two patches are rerolls from v2 of this series.\n\n[PATCH v3 1/3] read-cache.c: Handle long filenames correctly\n[PATCH v3 2/3] Strip namelen out of ce_flags into a ce_namelen field\n[PATCH v3 3/3] Replace strlen() with ce_namelen()\n\nbuiltin/apply.c           |    3 ++-\nbuiltin/blame.c           |    3 ++-\nbuiltin/checkout.c        |    3 ++-\nbuiltin/update-index.c    |    9 ++++++---\ncache.h                   |   19 ++++++-------------\nread-cache.c              |   62 +++++++++++++++++++++++++++++++++++++++-----------------------\nt/t0007-long-filenames.sh |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\ntree.c                    |    7 ++++---\nunpack-trees.c            |    5 +++--\n9 files changed, 126 insertions(+), 47 deletions(-)\n"},{"id":"194895","messageId":"1341998558-8502-2-git-send-email-t.gummerer@gmail.com","threadId":"30949","inReplyTo":"1341998558-8502-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v3 1/3] read-cache.c: Handle long filenames correctly","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2012-07-11T09:22:36Z","receivedAt":"2012-07-11T09:22:36Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Make git handle long file/path names (> 4096 characters) correctly.\n\nThere is a bug in the current version, which causes very long\nfile/pathnames to be handled incorrectly, or not even added to\nthe index, if they share the first 4096 characters.\n\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n read-cache.c              |    9 +++++--\n t/t0007-long-filenames.sh |   62 +++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 69 insertions(+), 2 deletions(-)\n create mode 100755 t/t0007-long-filenames.sh\n\ndiff --git a/read-cache.c b/read-cache.c\nindex ef355cc..4c6bf5f 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -399,8 +399,13 @@ int cache_name_compare(const char *name1, int flags1, const char *name2, int fla\n {\n \tint len1 = flags1 & CE_NAMEMASK;\n \tint len2 = flags2 & CE_NAMEMASK;\n-\tint len = len1 < len2 ? len1 : len2;\n-\tint cmp;\n+\tint len, cmp;\n+\n+\tif (len1 >= CE_NAMEMASK)\n+\t\tlen1 = strlen(name1 + CE_NAMEMASK) + CE_NAMEMASK;\n+\tif (len2 >= CE_NAMEMASK)\n+\t\tlen2 = strlen(name2 + CE_NAMEMASK) + CE_NAMEMASK;\n+\tlen = len1 < len2 ? len1 : len2;\n \n \tcmp = memcmp(name1, name2, len);\n \tif (cmp)\ndiff --git a/t/t0007-long-filenames.sh b/t/t0007-long-filenames.sh\nnew file mode 100755\nindex 0000000..2cf6035\n--- /dev/null\n+++ b/t/t0007-long-filenames.sh\n@@ -0,0 +1,62 @@\n+\n+#!/bin/sh\n+\n+test_description='very long file names in the index handled correctly'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\tgit init &&\n+\n+\ta=a && # 1\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 16\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 256\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 4096\n+\ta=${a}q &&\n+\n+\t>path1 &&\n+\tgit update-index --add path1 &&\n+\t(\n+\t\tgit ls-files -s path1 |\n+\t\tsed -e \"s/\t.*/\t/\" |\n+\t\ttr -d \"\\012\"\n+\t\techo \"$a\"\n+\t) | git update-index --index-info &&\n+\n+\ta=a && # 1\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 16\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 256\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 4096\n+\ta=${a}e &&\n+\n+\t>path2 &&\n+\tgit update-index --add path2 &&\n+\t(\n+\t\tgit ls-files -s path2 |\n+\t\tsed -e \"s/\t.*/\t/\" |\n+\t\ttr -d \"\\012\"\n+\t\techo \"$a\"\n+\t) | git update-index --index-info &&\n+\tlen=$(git ls-files \"a*\" | wc -c) &&\n+\ttest $len = 8196\n+'\n+\n+test_expect_success 'validate git ls-files output for a known tree' '\n+\tgit ls-files \"a*\" >current &&\n+\ta=a && # 1\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 16\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 256\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 4096\n+\ta=${a}e &&\n+\techo $a >expected &&\n+\n+\ta=a && # 1\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 16\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 256\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 4096\n+\ta=${a}q &&\n+\techo $a >>expected &&\n+\n+\ttest_cmp expected current\n+'\n+\n+test_done\n-- \n1.7.10.GIT\n"},{"id":"194898","messageId":"1341998558-8502-3-git-send-email-t.gummerer@gmail.com","threadId":"30949","inReplyTo":"1341998558-8502-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v3 2/3] Strip namelen out of ce_flags into a ce_namelen field","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2012-07-11T09:22:37Z","receivedAt":"2012-07-11T09:22:37Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Strip the name length from the ce_flags field and move it\ninto its own ce_namelen field in struct cache_entry. This\nwill both give us a tiny bit of a performance enhancement\nwhen working with long pathnames and is a refactoring for\nmore readability of the code.\n\nIt enhances readability, by making it more clear what\nis a flag, and where the length is stored and make it clear\nwhich functions use stages in comparisions and which only\nuse the length.\n\nIt also makes CE_NAMEMASK private, so that users don't\nmistakenly write the name length in the flags.\n\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n builtin/apply.c        |    3 ++-\n builtin/blame.c        |    3 ++-\n builtin/checkout.c     |    3 ++-\n builtin/update-index.c |    9 ++++---\n cache.h                |   19 +++++---------\n read-cache.c           |   67 ++++++++++++++++++++++++++++--------------------\n tree.c                 |    7 ++---\n unpack-trees.c         |    3 ++-\n 8 files changed, 63 insertions(+), 51 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex dda9ea0..347633c 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -3502,7 +3502,8 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n \tce = xcalloc(1, ce_size);\n \tmemcpy(ce->name, path, namelen);\n \tce->ce_mode = create_ce_mode(mode);\n-\tce->ce_flags = namelen;\n+\tce->ce_flags = create_ce_flags(0);\n+\tce->ce_namelen = namelen;\n \tif (S_ISGITLINK(mode)) {\n \t\tconst char *s = buf;\n \ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 24d3dd5..a28004a 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2153,7 +2153,8 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \tce = xcalloc(1, size);\n \thashcpy(ce->sha1, origin->blob_sha1);\n \tmemcpy(ce->name, path, len);\n-\tce->ce_flags = create_ce_flags(len, 0);\n+\tce->ce_flags = create_ce_flags(0);\n+\tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\n \tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n \ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 3ddda34..2dfa45b 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -73,7 +73,8 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,\n \thashcpy(ce->sha1, sha1);\n \tmemcpy(ce->name, base, baselen);\n \tmemcpy(ce->name + baselen, pathname, len - baselen);\n-\tce->ce_flags = create_ce_flags(len, 0) | CE_UPDATE;\n+\tce->ce_flags = create_ce_flags(0) | CE_UPDATE;\n+\tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\n \tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);\n \treturn 0;\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 5f038d6..e747def 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -95,7 +95,8 @@ static int add_one_path(struct cache_entry *old, const char *path, int len, stru\n \tsize = cache_entry_size(len);\n \tce = xcalloc(1, size);\n \tmemcpy(ce->name, path, len);\n-\tce->ce_flags = len;\n+\tce->ce_flags = create_ce_flags(0);\n+\tce->ce_namelen = len;\n \tfill_stat_cache_info(ce, st);\n \tce->ce_mode = ce_mode_from_stat(old, st->st_mode);\n \n@@ -235,7 +236,8 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,\n \n \thashcpy(ce->sha1, sha1);\n \tmemcpy(ce->name, path, len);\n-\tce->ce_flags = create_ce_flags(len, stage);\n+\tce->ce_flags = create_ce_flags(stage);\n+\tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\n \tif (assume_unchanged)\n \t\tce->ce_flags |= CE_VALID;\n@@ -433,7 +435,8 @@ static struct cache_entry *read_one_ent(const char *which,\n \n \thashcpy(ce->sha1, sha1);\n \tmemcpy(ce->name, path, namelen);\n-\tce->ce_flags = create_ce_flags(namelen, stage);\n+\tce->ce_flags = create_ce_flags(stage);\n+\tce->ce_namelen = namelen;\n \tce->ce_mode = create_ce_mode(mode);\n \treturn ce;\n }\ndiff --git a/cache.h b/cache.h\nindex cc5048c..22d9484 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -128,13 +128,13 @@ struct cache_entry {\n \tunsigned int ce_gid;\n \tunsigned int ce_size;\n \tunsigned int ce_flags;\n+\tunsigned int ce_namelen;\n \tunsigned char sha1[20];\n \tstruct cache_entry *next;\n \tstruct cache_entry *dir_next;\n \tchar name[FLEX_ARRAY]; /* more */\n };\n \n-#define CE_NAMEMASK  (0x0fff)\n #define CE_STAGEMASK (0x3000)\n #define CE_EXTENDED  (0x4000)\n #define CE_VALID     (0x8000)\n@@ -198,21 +198,12 @@ static inline void copy_cache_entry(struct cache_entry *dst, struct cache_entry\n \tdst->ce_flags = (dst->ce_flags & ~CE_STATE_MASK) | state;\n }\n \n-static inline unsigned create_ce_flags(size_t len, unsigned stage)\n+static inline unsigned create_ce_flags(unsigned stage)\n {\n-\tif (len >= CE_NAMEMASK)\n-\t\tlen = CE_NAMEMASK;\n-\treturn (len | (stage << CE_STAGESHIFT));\n-}\n-\n-static inline size_t ce_namelen(const struct cache_entry *ce)\n-{\n-\tsize_t len = ce->ce_flags & CE_NAMEMASK;\n-\tif (len < CE_NAMEMASK)\n-\t\treturn len;\n-\treturn strlen(ce->name + CE_NAMEMASK) + CE_NAMEMASK;\n+\treturn (stage << CE_STAGESHIFT);\n }\n \n+#define ce_namelen(ce) ((ce)->ce_namelen)\n #define ce_size(ce) cache_entry_size(ce_namelen(ce))\n #define ce_stage(ce) ((CE_STAGEMASK & (ce)->ce_flags) >> CE_STAGESHIFT)\n #define ce_uptodate(ce) ((ce)->ce_flags & CE_UPTODATE)\n@@ -448,6 +439,7 @@ extern int discard_index(struct index_state *);\n extern int unmerged_index(const struct index_state *);\n extern int verify_path(const char *path);\n extern struct cache_entry *index_name_exists(struct index_state *istate, const char *name, int namelen, int igncase);\n+extern int index_name_stage_pos(const struct index_state *, const char *name, int namelen, int stage);\n extern int index_name_pos(const struct index_state *, const char *name, int namelen);\n #define ADD_CACHE_OK_TO_ADD 1\t\t/* Ok to add */\n #define ADD_CACHE_OK_TO_REPLACE 2\t/* Ok to replace file/directory */\n@@ -857,6 +849,7 @@ extern int validate_headref(const char *ref);\n extern int base_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);\n extern int df_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);\n extern int cache_name_compare(const char *name1, int len1, const char *name2, int len2);\n+extern int cache_name_stage_compare(const char *name1, int len1, int stage1, const char *name2, int len2, int stage2);\n \n extern void *read_object_with_reference(const unsigned char *sha1,\n \t\t\t\t\tconst char *required_type,\ndiff --git a/read-cache.c b/read-cache.c\nindex 4c6bf5f..ac13bca 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -17,6 +17,10 @@\n \n static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);\n \n+/* Mask for the name length in ce_flags in the on-disk index */\n+\n+#define CE_NAMEMASK  (0x0fff)\n+\n /* Index extensions.\n  *\n  * The first letter should be 'A'..'Z' for extensions that are not\n@@ -54,8 +58,8 @@ void rename_index_entry_at(struct index_state *istate, int nr, const char *new_n\n \n \tnew = xmalloc(cache_entry_size(namelen));\n \tcopy_cache_entry(new, old);\n-\tnew->ce_flags &= ~(CE_STATE_MASK | CE_NAMEMASK);\n-\tnew->ce_flags |= (namelen >= CE_NAMEMASK ? CE_NAMEMASK : namelen);\n+\tnew->ce_flags &= ~CE_STATE_MASK;\n+\tnew->ce_namelen = namelen;\n \tmemcpy(new->name, new_name, namelen + 1);\n \n \tcache_tree_invalidate_path(istate->cache_tree, old->name);\n@@ -395,17 +399,10 @@ int df_name_compare(const char *name1, int len1, int mode1,\n \treturn c1 - c2;\n }\n \n-int cache_name_compare(const char *name1, int flags1, const char *name2, int flags2)\n+int cache_name_stage_compare(const char *name1, int len1, int stage1, const char *name2, int len2, int stage2)\n {\n-\tint len1 = flags1 & CE_NAMEMASK;\n-\tint len2 = flags2 & CE_NAMEMASK;\n-\tint len, cmp;\n-\n-\tif (len1 >= CE_NAMEMASK)\n-\t\tlen1 = strlen(name1 + CE_NAMEMASK) + CE_NAMEMASK;\n-\tif (len2 >= CE_NAMEMASK)\n-\t\tlen2 = strlen(name2 + CE_NAMEMASK) + CE_NAMEMASK;\n-\tlen = len1 < len2 ? len1 : len2;\n+\tint len = len1 < len2 ? len1 : len2;\n+\tint cmp;\n \n \tcmp = memcmp(name1, name2, len);\n \tif (cmp)\n@@ -415,18 +412,19 @@ int cache_name_compare(const char *name1, int flags1, const char *name2, int fla\n \tif (len1 > len2)\n \t\treturn 1;\n \n-\t/* Compare stages  */\n-\tflags1 &= CE_STAGEMASK;\n-\tflags2 &= CE_STAGEMASK;\n-\n-\tif (flags1 < flags2)\n+\tif (stage1 < stage2)\n \t\treturn -1;\n-\tif (flags1 > flags2)\n+\tif (stage1 > stage2)\n \t\treturn 1;\n \treturn 0;\n }\n \n-int index_name_pos(const struct index_state *istate, const char *name, int namelen)\n+int cache_name_compare(const char *name1, int len1, const char *name2, int len2)\n+{\n+\treturn cache_name_stage_compare(name1, len1, 0, name2, len2, 0);\n+}\n+\n+int index_name_stage_pos(const struct index_state *istate, const char *name, int namelen, int stage)\n {\n \tint first, last;\n \n@@ -435,7 +433,7 @@ int index_name_pos(const struct index_state *istate, const char *name, int namel\n \twhile (last > first) {\n \t\tint next = (last + first) >> 1;\n \t\tstruct cache_entry *ce = istate->cache[next];\n-\t\tint cmp = cache_name_compare(name, namelen, ce->name, ce->ce_flags);\n+\t\tint cmp = cache_name_stage_compare(name, namelen, stage, ce->name, ce_namelen(ce), ce_stage(ce));\n \t\tif (!cmp)\n \t\t\treturn next;\n \t\tif (cmp < 0) {\n@@ -447,6 +445,11 @@ int index_name_pos(const struct index_state *istate, const char *name, int namel\n \treturn -first-1;\n }\n \n+int index_name_pos(const struct index_state *istate, const char *name, int namelen)\n+{\n+\treturn index_name_stage_pos(istate, name, namelen, 0);\n+}\n+\n /* Remove entry, return true if there are more entries to go.. */\n int remove_index_entry_at(struct index_state *istate, int pos)\n {\n@@ -586,7 +589,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \tsize = cache_entry_size(namelen);\n \tce = xcalloc(1, size);\n \tmemcpy(ce->name, path, namelen);\n-\tce->ce_flags = namelen;\n+\tce->ce_namelen = namelen;\n \tif (!intent_only)\n \t\tfill_stat_cache_info(ce, st);\n \telse\n@@ -688,7 +691,8 @@ struct cache_entry *make_cache_entry(unsigned int mode,\n \n \thashcpy(ce->sha1, sha1);\n \tmemcpy(ce->name, path, len);\n-\tce->ce_flags = create_ce_flags(len, stage);\n+\tce->ce_flags = create_ce_flags(stage);\n+\tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\n \n \tif (refresh)\n@@ -825,7 +829,7 @@ static int has_dir_name(struct index_state *istate,\n \t\t}\n \t\tlen = slash - name;\n \n-\t\tpos = index_name_pos(istate, name, create_ce_flags(len, stage));\n+\t\tpos = index_name_stage_pos(istate, name, len, stage);\n \t\tif (pos >= 0) {\n \t\t\t/*\n \t\t\t * Found one, but not so fast.  This could\n@@ -915,7 +919,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n \tint new_only = option & ADD_CACHE_NEW_ONLY;\n \n \tcache_tree_invalidate_path(istate->cache_tree, ce->name);\n-\tpos = index_name_pos(istate, ce->name, ce->ce_flags);\n+\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n \n \t/* existing match? Just replace it. */\n \tif (pos >= 0) {\n@@ -947,7 +951,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n \t\tif (!ok_to_replace)\n \t\t\treturn error(\"'%s' appears as both a file and as a directory\",\n \t\t\t\t     ce->name);\n-\t\tpos = index_name_pos(istate, ce->name, ce->ce_flags);\n+\t\tpos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));\n \t\tpos = -pos-1;\n \t}\n \treturn pos + 1;\n@@ -1324,7 +1328,8 @@ static struct cache_entry *cache_entry_from_ondisk(struct ondisk_cache_entry *on\n \tce->ce_uid   = ntoh_l(ondisk->uid);\n \tce->ce_gid   = ntoh_l(ondisk->gid);\n \tce->ce_size  = ntoh_l(ondisk->size);\n-\tce->ce_flags = flags;\n+\tce->ce_flags = flags & ~CE_NAMEMASK;\n+\tce->ce_namelen = len;\n \thashcpy(ce->sha1, ondisk->sha1);\n \tmemcpy(ce->name, name, len);\n \tce->name[len] = '\\0';\n@@ -1651,6 +1656,8 @@ static void ce_smudge_racily_clean_entry(struct cache_entry *ce)\n static char *copy_cache_entry_to_ondisk(struct ondisk_cache_entry *ondisk,\n \t\t\t\t       struct cache_entry *ce)\n {\n+\tshort flags;\n+\n \tondisk->ctime.sec = htonl(ce->ce_ctime.sec);\n \tondisk->mtime.sec = htonl(ce->ce_mtime.sec);\n \tondisk->ctime.nsec = htonl(ce->ce_ctime.nsec);\n@@ -1662,7 +1669,10 @@ static char *copy_cache_entry_to_ondisk(struct ondisk_cache_entry *ondisk,\n \tondisk->gid  = htonl(ce->ce_gid);\n \tondisk->size = htonl(ce->ce_size);\n \thashcpy(ondisk->sha1, ce->sha1);\n-\tondisk->flags = htons(ce->ce_flags);\n+\n+\tflags = ce->ce_flags;\n+\tflags |= (ce_namelen(ce) >= CE_NAMEMASK ? CE_NAMEMASK : ce_namelen(ce));\n+\tondisk->flags = htons(flags);\n \tif (ce->ce_flags & CE_EXTENDED) {\n \t\tstruct ondisk_cache_entry_extended *ondisk2;\n \t\tondisk2 = (struct ondisk_cache_entry_extended *)ondisk;\n@@ -1850,7 +1860,8 @@ int read_index_unmerged(struct index_state *istate)\n \t\tsize = cache_entry_size(len);\n \t\tnew_ce = xcalloc(1, size);\n \t\tmemcpy(new_ce->name, ce->name, len);\n-\t\tnew_ce->ce_flags = create_ce_flags(len, 0) | CE_CONFLICTED;\n+\t\tnew_ce->ce_flags = create_ce_flags(0) | CE_CONFLICTED;\n+\t\tnew_ce->ce_namelen = len;\n \t\tnew_ce->ce_mode = ce->ce_mode;\n \t\tif (add_index_entry(istate, new_ce, 0))\n \t\t\treturn error(\"%s: cannot drop to stage #0\",\ndiff --git a/tree.c b/tree.c\nindex 676e9f7..62fed63 100644\n--- a/tree.c\n+++ b/tree.c\n@@ -22,7 +22,8 @@ static int read_one_entry_opt(const unsigned char *sha1, const char *base, int b\n \tce = xcalloc(1, size);\n \n \tce->ce_mode = create_ce_mode(mode);\n-\tce->ce_flags = create_ce_flags(baselen + len, stage);\n+\tce->ce_flags = create_ce_flags(stage);\n+\tce->ce_namelen = baselen + len;\n \tmemcpy(ce->name, base, baselen);\n \tmemcpy(ce->name + baselen, pathname, len+1);\n \thashcpy(ce->sha1, sha1);\n@@ -133,8 +134,8 @@ static int cmp_cache_name_compare(const void *a_, const void *b_)\n \n \tce1 = *((const struct cache_entry **)a_);\n \tce2 = *((const struct cache_entry **)b_);\n-\treturn cache_name_compare(ce1->name, ce1->ce_flags,\n-\t\t\t\t  ce2->name, ce2->ce_flags);\n+\treturn cache_name_stage_compare(ce1->name, ce1->ce_namelen, ce_stage(ce1),\n+\t\t\t\t  ce2->name, ce2->ce_namelen, ce_stage(ce2));\n }\n \n int read_tree(struct tree *tree, int stage, struct pathspec *match)\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex ad40109..9981dd3 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -539,7 +539,8 @@ static struct cache_entry *create_ce_entry(const struct traverse_info *info, con\n \tstruct cache_entry *ce = xcalloc(1, cache_entry_size(len));\n \n \tce->ce_mode = create_ce_mode(n->mode);\n-\tce->ce_flags = create_ce_flags(len, stage);\n+\tce->ce_flags = create_ce_flags(stage);\n+\tce->ce_namelen = len;\n \thashcpy(ce->sha1, n->sha1);\n \tmake_traverse_path(ce->name, info, n);\n \n-- \n1.7.10.GIT\n"},{"id":"194897","messageId":"1341998558-8502-4-git-send-email-t.gummerer@gmail.com","threadId":"30949","inReplyTo":"1341998558-8502-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v3 3/3] Replace strlen() with ce_namelen()","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2012-07-11T09:22:38Z","receivedAt":"2012-07-11T09:22:38Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Replace strlen(ce->name) with ce_namelen() in a couple\nof places which gives us some additional bits of\nperformance.\n\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n read-cache.c   |    4 ++--\n unpack-trees.c |    2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex ac13bca..2f8159f 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1128,7 +1128,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\t\tcontinue;\n \n \t\tif (pathspec &&\n-\t\t    !match_pathspec(pathspec, ce->name, strlen(ce->name), 0, seen))\n+\t\t    !match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))\n \t\t\tfiltered = 1;\n \n \t\tif (ce_stage(ce)) {\n@@ -1856,7 +1856,7 @@ int read_index_unmerged(struct index_state *istate)\n \t\tif (!ce_stage(ce))\n \t\t\tcontinue;\n \t\tunmerged = 1;\n-\t\tlen = strlen(ce->name);\n+\t\tlen = ce_namelen(ce);\n \t\tsize = cache_entry_size(len);\n \t\tnew_ce = xcalloc(1, size);\n \t\tmemcpy(new_ce->name, ce->name, len);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 9981dd3..abd0988 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1289,7 +1289,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce,\n \t * First let's make sure we do not have a local modification\n \t * in that directory.\n \t */\n-\tnamelen = strlen(ce->name);\n+\tnamelen = ce_namelen(ce);\n \tfor (i = locate_in_src_index(ce, o);\n \t     i < o->src_index->cache_nr;\n \t     i++) {\n-- \n1.7.10.GIT\n"},{"id":"194899","messageId":"CACsJy8A880GeyTXy9jCa6vhD-ajL29qimf5Mf94eCVjAOb-TMg@mail.gmail.com","threadId":"30949","inReplyTo":"1341998558-8502-2-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH v3 1/3] read-cache.c: Handle long filenames correctly","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-07-11T11:34:21Z","receivedAt":"2012-07-11T11:34:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jul 11, 2012 at 4:22 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:\n> Make git handle long file/path names (> 4096 characters) correctly.\n>\n> There is a bug in the current version, which causes very long\n> file/pathnames to be handled incorrectly, or not even added to\n> the index, if they share the first 4096 characters.\n\nThe patch looks correct to me though we're stepping on the border\nhere. Linux's PATH_MAX is 4k and Git already has hard time dealing\nwith >=4k paths (even when a single path component is less than 4k).\n\n> +       >path1 &&\n> +       git update-index --add path1 &&\n> +       (\n> +               git ls-files -s path1 |\n> +               sed -e \"s/      .*/     /\" |\n> +               tr -d \"\\012\"\n> +               echo \"$a\"\n> +       ) | git update-index --index-info &&\n> +\n\nor\n\nBLOB=$(git hash-object -w -t blob --stdin </dev/null)\ngit update-index --cacheinfo 100644 $BLOB $a\n\nI don't think git cares much in these tests and using empty tree sha-1\nmay even work (git recognizes that sha-1 automatically), but it may\nhurt the reader..\n-- \nDuy\n"},{"id":"194918","messageId":"7vsjcyfehg.fsf@alter.siamese.dyndns.org","threadId":"30949","inReplyTo":"7vy5mtlebz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC v2 1/2] Strip namelen out of ce_flags into a ce_namelen field","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-11T16:29:31Z","receivedAt":"2012-07-11T16:29:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> @@ -395,10 +395,8 @@ int df_name_compare(const char *name1, int len1, int mode1,\n>>  \treturn c1 - c2;\n>>  }\n>>  \n>> -int cache_name_compare(const char *name1, int flags1, const char *name2, int flags2)\n>> +int cache_name_stage_compare(const char *name1, int stage1, int len1, const char *name2, int stage2, int len2)\n>>  {\n>> -\tint len1 = flags1 & CE_NAMEMASK;\n>> -\tint len2 = flags2 & CE_NAMEMASK;\n>>  \tint len = len1 < len2 ? len1 : len2;\n>>  \tint cmp;\n>\n> Isn't this a _BUGFIX_?  It appears to me that the original code\n> would only compare the first 4k bytes and ignore the rest, if two\n> cache entries, both with overlong names, are compared.  Care to come\n> up with a test case to demonstrate the breakage and a bugfix without\n> the remainder of this patch, to be applied to 'master' and older\n> maintenance releases?\n\nPerhaps something like this (based on v1.7.0.9 as this may deserve\nto go to older maintenance releases).\n\n-- >8 --\nSubject: [PATCH] cache_name_compare(): do not truncate while comparing paths\n\nWe failed to use ce_namelen() equivalent and instead only compared\nup to the CE_NAMEMASK bytes by mistake.  Adding an overlong path\nthat shares the same common prefix as an existing entry in the index\ndid not add a new entry, but instead replaced the existing one, as\nthe result.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n read-cache.c             | 13 +++++++++----\n t/t3006-ls-files-long.sh | 39 +++++++++++++++++++++++++++++++++++++++\n 2 files changed, 48 insertions(+), 4 deletions(-)\n create mode 100755 t/t3006-ls-files-long.sh\n\ndiff --git a/read-cache.c b/read-cache.c\nindex f1f789b..0cd13aa 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -405,10 +405,15 @@ int df_name_compare(const char *name1, int len1, int mode1,\n \n int cache_name_compare(const char *name1, int flags1, const char *name2, int flags2)\n {\n-\tint len1 = flags1 & CE_NAMEMASK;\n-\tint len2 = flags2 & CE_NAMEMASK;\n-\tint len = len1 < len2 ? len1 : len2;\n-\tint cmp;\n+\tint len1, len2, len, cmp;\n+\n+\tlen1 = flags1 & CE_NAMEMASK;\n+\tif (CE_NAMEMASK <= len1)\n+\t\tlen1 = strlen(name1 + CE_NAMEMASK) + CE_NAMEMASK;\n+\tlen2 = flags2 & CE_NAMEMASK;\n+\tif (CE_NAMEMASK <= len2)\n+\t\tlen2 = strlen(name2 + CE_NAMEMASK) + CE_NAMEMASK;\n+\tlen = len1 < len2 ? len1 : len2;\n \n \tcmp = memcmp(name1, name2, len);\n \tif (cmp)\ndiff --git a/t/t3006-ls-files-long.sh b/t/t3006-ls-files-long.sh\nnew file mode 100755\nindex 0000000..202ad65\n--- /dev/null\n+++ b/t/t3006-ls-files-long.sh\n@@ -0,0 +1,39 @@\n+#!/bin/sh\n+\n+test_description='overly long paths'\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tp=filefilefilefilefilefilefilefile &&\n+\tp=$p$p$p$p$p$p$p$p$p$p$p$p$p$p$p$p &&\n+\tp=$p$p$p$p$p$p$p$p$p$p$p$p$p$p$p$p &&\n+\n+\tpath_a=${p}_a &&\n+\tpath_z=${p}_z &&\n+\n+\tblob_a=$(echo frotz | git hash-object -w --stdin) &&\n+\tblob_z=$(echo nitfol | git hash-object -w --stdin) &&\n+\n+\tpat=\"100644 %s 0\\t%s\\n\"\n+'\n+\n+test_expect_success 'overly-long path by itself is not a problem' '\n+\tprintf \"$pat\" \"$blob_a\" \"$path_a\" |\n+\tgit update-index --add --index-info &&\n+\techo \"$path_a\" >expect &&\n+\tgit ls-files >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'overly-long path does not replace another by mistake' '\n+\tprintf \"$pat\" \"$blob_a\" \"$path_a\" \"$blob_z\" \"$path_z\" |\n+\tgit update-index --add --index-info &&\n+\t(\n+\t\techo \"$path_a\"\n+\t\techo \"$path_z\"\n+\t) >expect &&\n+\tgit ls-files >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n1.7.11\n"},{"id":"194919","messageId":"7vobnmfea8.fsf@alter.siamese.dyndns.org","threadId":"30949","inReplyTo":"1341998558-8502-2-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH v3 1/3] read-cache.c: Handle long filenames correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-11T16:33:51Z","receivedAt":"2012-07-11T16:33:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer <t.gummerer@gmail.com> writes:\n\n> Make git handle long file/path names (> 4096 characters) correctly.\n>\n> There is a bug in the current version, which causes very long\n> file/pathnames to be handled incorrectly, or not even added to\n> the index, if they share the first 4096 characters.\n>\n> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n\nAhh, I guess I should have opened my mailbox before starting to look\ninto this myself ;-)\n"},{"id":"194920","messageId":"7vk3yafe7w.fsf@alter.siamese.dyndns.org","threadId":"30949","inReplyTo":"1341998558-8502-4-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH v3 3/3] Replace strlen() with ce_namelen()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-11T16:35:15Z","receivedAt":"2012-07-11T16:35:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks for resending; I've merged this independently to 'next' and\nwill merge it to 'master' soonish.  This one is trivially correct.\n"}]}