{"thread":{"id":"30345","subject":"[PATCH v2 01/18] get_ref_dir(): return early if directory cannot be read","startedAt":"2012-04-26T22:26:49Z","lastAt":"2012-05-04T07:24:44Z","messageCount":26,"participants":["mhagger@alum.mit.edu","Junio C Hamano","Michael Haggerty"],"isPatch":true,"patchVersion":2,"patchTotal":18},"messages":[{"id":"190167","messageId":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":null,"subject":"[PATCH v2 00/18] Read loose references lazily","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:26:49Z","receivedAt":"2012-04-26T22:26:49Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThis is the next installment of the ref-api saga.  The main result of\nthis patch series is to teach git to read loose references one\ndirectory at a time, only when they are needed.\n\nThis version of the patch series differs from v1 by avoiding the\nconversion of code from using (struct ref_dir *) to (struct ref_entry\n*).  As Junio rightly pointed out, the use of ref_entry as the data\ntype for dealing with directory entries was awkward and error-prone.\n\nThis version also changes how bits in ref_entry::flag are assigned,\nalso at Junio's suggestion.\n\nThe first nine patches are unchanged from v1 except for a trivial\nchange to a commit message.  These patches convert get_ref_dir() and\ndo_for_each_reflog() to use strbufs and to tighten up their\nspecifications.\n\nPatch 10 fixes a git_path() related bug in bisect.c.  A filename\nobtained from git_path() was being used long after it was created.\nWith the changes later in this patch series, the\nlazy-loose-reference-reading code will be activated while\ncheck_good_are_ancestors_of_bad() is still holding on to the filename.\nTherefore, this fix is a prerequisite to the later patches.\n\n[I am nervous that there might be analogous bugs elsewhere in the\ncode.  Since the lazy-loose-reference-reading code can now spring to\nlife in many situations where references are used, it might flush out\nproblems in sloppy code elsewhere.  But what can we do, short of\ngetting rid of get_pathname() once and for all?]\n\nPatch 11 Converts find_containing_dir() to use a strbuf.  It makes the\ncode a bit more straightforward but is otherwise not very interesting.\n\nThe rest of the patches teach the code to read loose references\nlazily.  This is done by setting a REF_INCOMPLETE bit in\nref_entry::flag for loose directory entries that haven't been read\nyet.  The entries are read when they are accessed, namely when\nget_ref_dir() is called to extract a ref_dir from a directory\nref_entry.\n\nThis patch series depends on mh/ref-api, which just recently made it\nto master.  It passes all tests.\n\nMichael Haggerty (18):\n  get_ref_dir(): return early if directory cannot be read\n  get_ref_dir(): use a strbuf to hold refname\n  get_ref_dir(): rename \"base\" parameter to \"dirname\"\n  get_ref_dir(): require that the dirname argument ends in '/'\n  refs.c: extract function search_for_subdir()\n  get_ref_dir(): take the containing directory as argument\n  do_for_each_reflog(): return early on error\n  do_for_each_reflog(): use a strbuf to hold logfile name\n  do_for_each_reflog(): reuse strbuf across recursive function calls\n  bisect: copy filename string obtained from git_path()\n  find_containing_dir(): use strbuf in implementation of this function\n  refs: wrap top-level ref_dirs in ref_entries\n  read_loose_refs(): rename function from get_ref_dir()\n  get_ref_dir(): add function for getting a ref_dir from a ref_entry\n  search_for_subdir(): return (ref_dir *) instead of (ref_entry *)\n  struct ref_dir: store a reference to the enclosing ref_cache\n  read_loose_refs(): eliminate ref_cache argument\n  refs: read loose references lazily\n\n bisect.c |    8 +-\n refs.c   |  378 ++++++++++++++++++++++++++++++++++++++++----------------------\n 2 files changed, 247 insertions(+), 139 deletions(-)\n\n-- \n1.7.10\n"},{"id":"190165","messageId":"1335479227-7877-2-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 01/18] get_ref_dir(): return early if directory cannot be read","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:26:50Z","receivedAt":"2012-04-26T22:26:50Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   85 +++++++++++++++++++++++++++++++++-------------------------------\n 1 file changed, 44 insertions(+), 41 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 09322fe..d539241 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -754,6 +754,9 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,\n {\n \tDIR *d;\n \tconst char *path;\n+\tstruct dirent *de;\n+\tint baselen;\n+\tchar *refname;\n \n \tif (*refs->name)\n \t\tpath = git_path_submodule(refs->name, \"%s\", base);\n@@ -761,55 +764,55 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,\n \t\tpath = git_path(\"%s\", base);\n \n \td = opendir(path);\n-\tif (d) {\n-\t\tstruct dirent *de;\n-\t\tint baselen = strlen(base);\n-\t\tchar *refname = xmalloc(baselen + 257);\n+\tif (!d)\n+\t\treturn;\n \n-\t\tmemcpy(refname, base, baselen);\n-\t\tif (baselen && base[baselen-1] != '/')\n-\t\t\trefname[baselen++] = '/';\n+\tbaselen = strlen(base);\n+\trefname = xmalloc(baselen + 257);\n \n-\t\twhile ((de = readdir(d)) != NULL) {\n-\t\t\tunsigned char sha1[20];\n-\t\t\tstruct stat st;\n-\t\t\tint flag;\n-\t\t\tint namelen;\n-\t\t\tconst char *refdir;\n+\tmemcpy(refname, base, baselen);\n+\tif (baselen && base[baselen-1] != '/')\n+\t\trefname[baselen++] = '/';\n \n-\t\t\tif (de->d_name[0] == '.')\n-\t\t\t\tcontinue;\n-\t\t\tnamelen = strlen(de->d_name);\n-\t\t\tif (namelen > 255)\n-\t\t\t\tcontinue;\n-\t\t\tif (has_extension(de->d_name, \".lock\"))\n-\t\t\t\tcontinue;\n-\t\t\tmemcpy(refname + baselen, de->d_name, namelen+1);\n-\t\t\trefdir = *refs->name\n-\t\t\t\t? git_path_submodule(refs->name, \"%s\", refname)\n-\t\t\t\t: git_path(\"%s\", refname);\n-\t\t\tif (stat(refdir, &st) < 0)\n-\t\t\t\tcontinue;\n-\t\t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\t\tget_ref_dir(refs, refname, dir);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (*refs->name) {\n-\t\t\t\thashclr(sha1);\n-\t\t\t\tflag = 0;\n-\t\t\t\tif (resolve_gitlink_ref(refs->name, refname, sha1) < 0) {\n-\t\t\t\t\thashclr(sha1);\n-\t\t\t\t\tflag |= REF_ISBROKEN;\n-\t\t\t\t}\n-\t\t\t} else if (read_ref_full(refname, sha1, 1, &flag)) {\n+\twhile ((de = readdir(d)) != NULL) {\n+\t\tunsigned char sha1[20];\n+\t\tstruct stat st;\n+\t\tint flag;\n+\t\tint namelen;\n+\t\tconst char *refdir;\n+\n+\t\tif (de->d_name[0] == '.')\n+\t\t\tcontinue;\n+\t\tnamelen = strlen(de->d_name);\n+\t\tif (namelen > 255)\n+\t\t\tcontinue;\n+\t\tif (has_extension(de->d_name, \".lock\"))\n+\t\t\tcontinue;\n+\t\tmemcpy(refname + baselen, de->d_name, namelen+1);\n+\t\trefdir = *refs->name\n+\t\t\t? git_path_submodule(refs->name, \"%s\", refname)\n+\t\t\t: git_path(\"%s\", refname);\n+\t\tif (stat(refdir, &st) < 0)\n+\t\t\tcontinue;\n+\t\tif (S_ISDIR(st.st_mode)) {\n+\t\t\tget_ref_dir(refs, refname, dir);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (*refs->name) {\n+\t\t\thashclr(sha1);\n+\t\t\tflag = 0;\n+\t\t\tif (resolve_gitlink_ref(refs->name, refname, sha1) < 0) {\n \t\t\t\thashclr(sha1);\n \t\t\t\tflag |= REF_ISBROKEN;\n \t\t\t}\n-\t\t\tadd_ref(dir, create_ref_entry(refname, sha1, flag, 1));\n+\t\t} else if (read_ref_full(refname, sha1, 1, &flag)) {\n+\t\t\thashclr(sha1);\n+\t\t\tflag |= REF_ISBROKEN;\n \t\t}\n-\t\tfree(refname);\n-\t\tclosedir(d);\n+\t\tadd_ref(dir, create_ref_entry(refname, sha1, flag, 1));\n \t}\n+\tfree(refname);\n+\tclosedir(d);\n }\n \n static struct ref_dir *get_loose_refs(struct ref_cache *refs)\n-- \n1.7.10\n"},{"id":"190181","messageId":"1335479227-7877-3-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 02/18] get_ref_dir(): use a strbuf to hold refname","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:26:51Z","receivedAt":"2012-04-26T22:26:51Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThis simplifies the bookkeeping and allows an (artificial) restriction\non refname component length to be removed.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   54 ++++++++++++++++++++++++++----------------------------\n 1 file changed, 26 insertions(+), 28 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex d539241..df98622 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -756,7 +756,7 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,\n \tconst char *path;\n \tstruct dirent *de;\n \tint baselen;\n-\tchar *refname;\n+\tstruct strbuf refname;\n \n \tif (*refs->name)\n \t\tpath = git_path_submodule(refs->name, \"%s\", base);\n@@ -768,50 +768,48 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,\n \t\treturn;\n \n \tbaselen = strlen(base);\n-\trefname = xmalloc(baselen + 257);\n-\n-\tmemcpy(refname, base, baselen);\n-\tif (baselen && base[baselen-1] != '/')\n-\t\trefname[baselen++] = '/';\n+\tstrbuf_init(&refname, baselen + 257);\n+\tstrbuf_add(&refname, base, baselen);\n+\tif (baselen && base[baselen-1] != '/') {\n+\t\tstrbuf_addch(&refname, '/');\n+\t\tbaselen++;\n+\t}\n \n \twhile ((de = readdir(d)) != NULL) {\n \t\tunsigned char sha1[20];\n \t\tstruct stat st;\n \t\tint flag;\n-\t\tint namelen;\n \t\tconst char *refdir;\n \n \t\tif (de->d_name[0] == '.')\n \t\t\tcontinue;\n-\t\tnamelen = strlen(de->d_name);\n-\t\tif (namelen > 255)\n-\t\t\tcontinue;\n \t\tif (has_extension(de->d_name, \".lock\"))\n \t\t\tcontinue;\n-\t\tmemcpy(refname + baselen, de->d_name, namelen+1);\n+\t\tstrbuf_addstr(&refname, de->d_name);\n \t\trefdir = *refs->name\n-\t\t\t? git_path_submodule(refs->name, \"%s\", refname)\n-\t\t\t: git_path(\"%s\", refname);\n-\t\tif (stat(refdir, &st) < 0)\n-\t\t\tcontinue;\n-\t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\tget_ref_dir(refs, refname, dir);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (*refs->name) {\n-\t\t\thashclr(sha1);\n-\t\t\tflag = 0;\n-\t\t\tif (resolve_gitlink_ref(refs->name, refname, sha1) < 0) {\n+\t\t\t? git_path_submodule(refs->name, \"%s\", refname.buf)\n+\t\t\t: git_path(\"%s\", refname.buf);\n+\t\tif (stat(refdir, &st) < 0) {\n+\t\t\t/* Silently ignore. */\n+\t\t} else if (S_ISDIR(st.st_mode)) {\n+\t\t\tget_ref_dir(refs, refname.buf, dir);\n+\t\t} else {\n+\t\t\tif (*refs->name) {\n+\t\t\t\thashclr(sha1);\n+\t\t\t\tflag = 0;\n+\t\t\t\tif (resolve_gitlink_ref(refs->name, refname.buf, sha1) < 0) {\n+\t\t\t\t\thashclr(sha1);\n+\t\t\t\t\tflag |= REF_ISBROKEN;\n+\t\t\t\t}\n+\t\t\t} else if (read_ref_full(refname.buf, sha1, 1, &flag)) {\n \t\t\t\thashclr(sha1);\n \t\t\t\tflag |= REF_ISBROKEN;\n \t\t\t}\n-\t\t} else if (read_ref_full(refname, sha1, 1, &flag)) {\n-\t\t\thashclr(sha1);\n-\t\t\tflag |= REF_ISBROKEN;\n+\t\t\tadd_ref(dir, create_ref_entry(refname.buf, sha1, flag, 1));\n \t\t}\n-\t\tadd_ref(dir, create_ref_entry(refname, sha1, flag, 1));\n+\t\tstrbuf_setlen(&refname, baselen);\n \t}\n-\tfree(refname);\n+\tstrbuf_release(&refname);\n \tclosedir(d);\n }\n \n-- \n1.7.10\n"},{"id":"190166","messageId":"1335479227-7877-4-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 03/18] get_ref_dir(): rename \"base\" parameter to \"dirname\"","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:26:52Z","receivedAt":"2012-04-26T22:26:52Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex df98622..dc2b4df 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -749,30 +749,30 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)\n \t\t\tcreate_ref_entry(refname, sha1, REF_ISPACKED, 1));\n }\n \n-static void get_ref_dir(struct ref_cache *refs, const char *base,\n+static void get_ref_dir(struct ref_cache *refs, const char *dirname,\n \t\t\tstruct ref_dir *dir)\n {\n \tDIR *d;\n \tconst char *path;\n \tstruct dirent *de;\n-\tint baselen;\n+\tint dirnamelen;\n \tstruct strbuf refname;\n \n \tif (*refs->name)\n-\t\tpath = git_path_submodule(refs->name, \"%s\", base);\n+\t\tpath = git_path_submodule(refs->name, \"%s\", dirname);\n \telse\n-\t\tpath = git_path(\"%s\", base);\n+\t\tpath = git_path(\"%s\", dirname);\n \n \td = opendir(path);\n \tif (!d)\n \t\treturn;\n \n-\tbaselen = strlen(base);\n-\tstrbuf_init(&refname, baselen + 257);\n-\tstrbuf_add(&refname, base, baselen);\n-\tif (baselen && base[baselen-1] != '/') {\n+\tdirnamelen = strlen(dirname);\n+\tstrbuf_init(&refname, dirnamelen + 257);\n+\tstrbuf_add(&refname, dirname, dirnamelen);\n+\tif (dirnamelen && dirname[dirnamelen-1] != '/') {\n \t\tstrbuf_addch(&refname, '/');\n-\t\tbaselen++;\n+\t\tdirnamelen++;\n \t}\n \n \twhile ((de = readdir(d)) != NULL) {\n@@ -807,7 +807,7 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,\n \t\t\t}\n \t\t\tadd_ref(dir, create_ref_entry(refname.buf, sha1, flag, 1));\n \t\t}\n-\t\tstrbuf_setlen(&refname, baselen);\n+\t\tstrbuf_setlen(&refname, dirnamelen);\n \t}\n \tstrbuf_release(&refname);\n \tclosedir(d);\n-- \n1.7.10\n"},{"id":"190168","messageId":"1335479227-7877-5-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 04/18] get_ref_dir(): require that the dirname argument ends in '/'","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:26:53Z","receivedAt":"2012-04-26T22:26:53Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThis removes some conditional code and makes it consistent with the\nway that direntry names are stored.  Please note that this function is\nnever used on the top-level .git directory; it is always called for\ndirectories at level .git/refs or deeper.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex dc2b4df..01fcdc7 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -749,13 +749,17 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)\n \t\t\tcreate_ref_entry(refname, sha1, REF_ISPACKED, 1));\n }\n \n+/*\n+ * Read the loose references for refs from the namespace dirname.\n+ * dirname must end with '/'.\n+ */\n static void get_ref_dir(struct ref_cache *refs, const char *dirname,\n \t\t\tstruct ref_dir *dir)\n {\n \tDIR *d;\n \tconst char *path;\n \tstruct dirent *de;\n-\tint dirnamelen;\n+\tint dirnamelen = strlen(dirname);\n \tstruct strbuf refname;\n \n \tif (*refs->name)\n@@ -767,13 +771,8 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,\n \tif (!d)\n \t\treturn;\n \n-\tdirnamelen = strlen(dirname);\n \tstrbuf_init(&refname, dirnamelen + 257);\n \tstrbuf_add(&refname, dirname, dirnamelen);\n-\tif (dirnamelen && dirname[dirnamelen-1] != '/') {\n-\t\tstrbuf_addch(&refname, '/');\n-\t\tdirnamelen++;\n-\t}\n \n \twhile ((de = readdir(d)) != NULL) {\n \t\tunsigned char sha1[20];\n@@ -792,6 +791,7 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,\n \t\tif (stat(refdir, &st) < 0) {\n \t\t\t/* Silently ignore. */\n \t\t} else if (S_ISDIR(st.st_mode)) {\n+\t\t\tstrbuf_addch(&refname, '/');\n \t\t\tget_ref_dir(refs, refname.buf, dir);\n \t\t} else {\n \t\t\tif (*refs->name) {\n@@ -816,7 +816,7 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,\n static struct ref_dir *get_loose_refs(struct ref_cache *refs)\n {\n \tif (!refs->did_loose) {\n-\t\tget_ref_dir(refs, \"refs\", &refs->loose);\n+\t\tget_ref_dir(refs, \"refs/\", &refs->loose);\n \t\trefs->did_loose = 1;\n \t}\n \treturn &refs->loose;\n-- \n1.7.10\n"},{"id":"190182","messageId":"1335479227-7877-6-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 05/18] refs.c: extract function search_for_subdir()","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:26:54Z","receivedAt":"2012-04-26T22:26:54Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   34 ++++++++++++++++++++++++----------\n 1 file changed, 24 insertions(+), 10 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 01fcdc7..5e51c10 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -277,6 +277,27 @@ static struct ref_entry *search_ref_dir(struct ref_dir *dir, const char *refname\n }\n \n /*\n+ * Search for a directory entry directly within dir (without\n+ * recursing).  Sort dir if necessary.  subdirname must be a directory\n+ * name (i.e., end in '/').  If mkdir is set, then create the\n+ * directory if it is missing; otherwise, return NULL if the desired\n+ * directory cannot be found.\n+ */\n+static struct ref_entry *search_for_subdir(struct ref_dir *dir,\n+\t\t\t\t\t   const char *subdirname, int mkdir)\n+{\n+\tstruct ref_entry *entry = search_ref_dir(dir, subdirname);\n+\tif (!entry) {\n+\t\tif (!mkdir)\n+\t\t\treturn NULL;\n+\t\tentry = create_dir_entry(subdirname);\n+\t\tadd_entry_to_dir(dir, entry);\n+\t}\n+\tassert(entry->flag & REF_DIR);\n+\treturn entry;\n+}\n+\n+/*\n  * If refname is a reference name, find the ref_dir within the dir\n  * tree that should hold refname.  If refname is a directory name\n  * (i.e., ends in '/'), then return that ref_dir itself.  dir must\n@@ -294,17 +315,10 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,\n \tfor (slash = strchr(refname_copy, '/'); slash; slash = strchr(slash + 1, '/')) {\n \t\tchar tmp = slash[1];\n \t\tslash[1] = '\\0';\n-\t\tentry = search_ref_dir(dir, refname_copy);\n-\t\tif (!entry) {\n-\t\t\tif (!mkdir) {\n-\t\t\t\tdir = NULL;\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t\tentry = create_dir_entry(refname_copy);\n-\t\t\tadd_entry_to_dir(dir, entry);\n-\t\t}\n+\t\tentry = search_for_subdir(dir, refname_copy, mkdir);\n \t\tslash[1] = tmp;\n-\t\tassert(entry->flag & REF_DIR);\n+\t\tif (!entry)\n+\t\t\tbreak;\n \t\tdir = &entry->u.subdir;\n \t}\n \n-- \n1.7.10\n"},{"id":"190172","messageId":"1335479227-7877-7-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 06/18] get_ref_dir(): take the containing directory as argument","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:26:55Z","receivedAt":"2012-04-26T22:26:55Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nPreviously, the \"dir\" argument to get_ref_dir() was a pointer to the\ntop-level ref_dir.  Change the function to expect a pointer to the\nref_dir corresponding to dirname.  This allows entries to be added\ndirectly to dir, without having to recurse through the reference trie\neach time (i.e., we can use add_entry_to_dir() instead of add_ref()).\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 5e51c10..8670c2e 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -765,7 +765,8 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)\n \n /*\n  * Read the loose references for refs from the namespace dirname.\n- * dirname must end with '/'.\n+ * dirname must end with '/'.  dir must be the directory entry\n+ * corresponding to dirname.\n  */\n static void get_ref_dir(struct ref_cache *refs, const char *dirname,\n \t\t\tstruct ref_dir *dir)\n@@ -806,7 +807,8 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,\n \t\t\t/* Silently ignore. */\n \t\t} else if (S_ISDIR(st.st_mode)) {\n \t\t\tstrbuf_addch(&refname, '/');\n-\t\t\tget_ref_dir(refs, refname.buf, dir);\n+\t\t\tget_ref_dir(refs, refname.buf,\n+\t\t\t\t    &search_for_subdir(dir, refname.buf, 1)->u.subdir);\n \t\t} else {\n \t\t\tif (*refs->name) {\n \t\t\t\thashclr(sha1);\n@@ -819,7 +821,8 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,\n \t\t\t\thashclr(sha1);\n \t\t\t\tflag |= REF_ISBROKEN;\n \t\t\t}\n-\t\t\tadd_ref(dir, create_ref_entry(refname.buf, sha1, flag, 1));\n+\t\t\tadd_entry_to_dir(dir,\n+\t\t\t\t\t create_ref_entry(refname.buf, sha1, flag, 1));\n \t\t}\n \t\tstrbuf_setlen(&refname, dirnamelen);\n \t}\n@@ -830,7 +833,8 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,\n static struct ref_dir *get_loose_refs(struct ref_cache *refs)\n {\n \tif (!refs->did_loose) {\n-\t\tget_ref_dir(refs, \"refs/\", &refs->loose);\n+\t\tget_ref_dir(refs, \"refs/\",\n+\t\t\t    &search_for_subdir(&refs->loose, \"refs/\", 1)->u.subdir);\n \t\trefs->did_loose = 1;\n \t}\n \treturn &refs->loose;\n-- \n1.7.10\n"},{"id":"190169","messageId":"1335479227-7877-8-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 07/18] do_for_each_reflog(): return early on error","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:26:56Z","receivedAt":"2012-04-26T22:26:56Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   70 ++++++++++++++++++++++++++++++++--------------------------------\n 1 file changed, 35 insertions(+), 35 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 8670c2e..1d25151 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2246,47 +2246,47 @@ static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)\n {\n \tDIR *d = opendir(git_path(\"logs/%s\", base));\n \tint retval = 0;\n+\tstruct dirent *de;\n+\tint baselen;\n+\tchar *log;\n \n-\tif (d) {\n-\t\tstruct dirent *de;\n-\t\tint baselen = strlen(base);\n-\t\tchar *log = xmalloc(baselen + 257);\n+\tif (!d)\n+\t\treturn *base ? errno : 0;\n \n-\t\tmemcpy(log, base, baselen);\n-\t\tif (baselen && base[baselen-1] != '/')\n-\t\t\tlog[baselen++] = '/';\n+\tbaselen = strlen(base);\n+\tlog = xmalloc(baselen + 257);\n+\tmemcpy(log, base, baselen);\n+\tif (baselen && base[baselen-1] != '/')\n+\t\tlog[baselen++] = '/';\n \n-\t\twhile ((de = readdir(d)) != NULL) {\n-\t\t\tstruct stat st;\n-\t\t\tint namelen;\n+\twhile ((de = readdir(d)) != NULL) {\n+\t\tstruct stat st;\n+\t\tint namelen;\n \n-\t\t\tif (de->d_name[0] == '.')\n-\t\t\t\tcontinue;\n-\t\t\tnamelen = strlen(de->d_name);\n-\t\t\tif (namelen > 255)\n-\t\t\t\tcontinue;\n-\t\t\tif (has_extension(de->d_name, \".lock\"))\n-\t\t\t\tcontinue;\n-\t\t\tmemcpy(log + baselen, de->d_name, namelen+1);\n-\t\t\tif (stat(git_path(\"logs/%s\", log), &st) < 0)\n-\t\t\t\tcontinue;\n-\t\t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\t\tretval = do_for_each_reflog(log, fn, cb_data);\n-\t\t\t} else {\n-\t\t\t\tunsigned char sha1[20];\n-\t\t\t\tif (read_ref_full(log, sha1, 0, NULL))\n-\t\t\t\t\tretval = error(\"bad ref for %s\", log);\n-\t\t\t\telse\n-\t\t\t\t\tretval = fn(log, sha1, 0, cb_data);\n-\t\t\t}\n-\t\t\tif (retval)\n-\t\t\t\tbreak;\n+\t\tif (de->d_name[0] == '.')\n+\t\t\tcontinue;\n+\t\tnamelen = strlen(de->d_name);\n+\t\tif (namelen > 255)\n+\t\t\tcontinue;\n+\t\tif (has_extension(de->d_name, \".lock\"))\n+\t\t\tcontinue;\n+\t\tmemcpy(log + baselen, de->d_name, namelen+1);\n+\t\tif (stat(git_path(\"logs/%s\", log), &st) < 0)\n+\t\t\tcontinue;\n+\t\tif (S_ISDIR(st.st_mode)) {\n+\t\t\tretval = do_for_each_reflog(log, fn, cb_data);\n+\t\t} else {\n+\t\t\tunsigned char sha1[20];\n+\t\t\tif (read_ref_full(log, sha1, 0, NULL))\n+\t\t\t\tretval = error(\"bad ref for %s\", log);\n+\t\t\telse\n+\t\t\t\tretval = fn(log, sha1, 0, cb_data);\n \t\t}\n-\t\tfree(log);\n-\t\tclosedir(d);\n+\t\tif (retval)\n+\t\t\tbreak;\n \t}\n-\telse if (*base)\n-\t\treturn errno;\n+\tfree(log);\n+\tclosedir(d);\n \treturn retval;\n }\n \n-- \n1.7.10\n"},{"id":"190170","messageId":"1335479227-7877-9-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:26:57Z","receivedAt":"2012-04-26T22:26:57Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThis simplifies the bookkeeping and allows an (artificial) restriction\non refname component length to be removed.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   45 +++++++++++++++++++++++----------------------\n 1 file changed, 23 insertions(+), 22 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 1d25151..f43c255 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2248,44 +2248,45 @@ static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)\n \tint retval = 0;\n \tstruct dirent *de;\n \tint baselen;\n-\tchar *log;\n+\tstruct strbuf log;\n \n \tif (!d)\n \t\treturn *base ? errno : 0;\n \n \tbaselen = strlen(base);\n-\tlog = xmalloc(baselen + 257);\n-\tmemcpy(log, base, baselen);\n-\tif (baselen && base[baselen-1] != '/')\n-\t\tlog[baselen++] = '/';\n+\tstrbuf_init(&log, baselen + 257);\n+\tstrbuf_add(&log, base, baselen);\n+\tif (log.len && log.buf[log.len-1] != '/') {\n+\t\tstrbuf_addch(&log, '/');\n+\t\tbaselen++;\n+\t}\n \n \twhile ((de = readdir(d)) != NULL) {\n \t\tstruct stat st;\n-\t\tint namelen;\n \n \t\tif (de->d_name[0] == '.')\n \t\t\tcontinue;\n-\t\tnamelen = strlen(de->d_name);\n-\t\tif (namelen > 255)\n-\t\t\tcontinue;\n \t\tif (has_extension(de->d_name, \".lock\"))\n \t\t\tcontinue;\n-\t\tmemcpy(log + baselen, de->d_name, namelen+1);\n-\t\tif (stat(git_path(\"logs/%s\", log), &st) < 0)\n-\t\t\tcontinue;\n-\t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\tretval = do_for_each_reflog(log, fn, cb_data);\n+\t\tstrbuf_addstr(&log, de->d_name);\n+\t\tif (stat(git_path(\"logs/%s\", log.buf), &st) < 0) {\n+\t\t\t/* Silently ignore. */\n \t\t} else {\n-\t\t\tunsigned char sha1[20];\n-\t\t\tif (read_ref_full(log, sha1, 0, NULL))\n-\t\t\t\tretval = error(\"bad ref for %s\", log);\n-\t\t\telse\n-\t\t\t\tretval = fn(log, sha1, 0, cb_data);\n+\t\t\tif (S_ISDIR(st.st_mode)) {\n+\t\t\t\tretval = do_for_each_reflog(log.buf, fn, cb_data);\n+\t\t\t} else {\n+\t\t\t\tunsigned char sha1[20];\n+\t\t\t\tif (read_ref_full(log.buf, sha1, 0, NULL))\n+\t\t\t\t\tretval = error(\"bad ref for %s\", log.buf);\n+\t\t\t\telse\n+\t\t\t\t\tretval = fn(log.buf, sha1, 0, cb_data);\n+\t\t\t}\n+\t\t\tif (retval)\n+\t\t\t\tbreak;\n \t\t}\n-\t\tif (retval)\n-\t\t\tbreak;\n+\t\tstrbuf_setlen(&log, baselen);\n \t}\n-\tfree(log);\n+\tstrbuf_release(&log);\n \tclosedir(d);\n \treturn retval;\n }\n-- \n1.7.10\n"},{"id":"190180","messageId":"1335479227-7877-10-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 09/18] do_for_each_reflog(): reuse strbuf across recursive function calls","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:26:58Z","receivedAt":"2012-04-26T22:26:58Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThis saves some memory allocations.  Also require that the name\nargument end in slash, which removes some extra bookkeeping.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   45 +++++++++++++++++++++++----------------------\n 1 file changed, 23 insertions(+), 22 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex f43c255..989c10d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2242,24 +2242,20 @@ int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_dat\n \treturn for_each_recent_reflog_ent(refname, fn, 0, cb_data);\n }\n \n-static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)\n+/*\n+ * Call fn for each reflog in the namespace indicated by name.  name\n+ * must be empty or end with '/'.  Name will be used as a scratch\n+ * space, but its contents will be restored before return.\n+ */\n+static int do_for_each_reflog(struct strbuf *name, each_ref_fn fn, void *cb_data)\n {\n-\tDIR *d = opendir(git_path(\"logs/%s\", base));\n+\tDIR *d = opendir(git_path(\"logs/%s\", name->buf));\n \tint retval = 0;\n \tstruct dirent *de;\n-\tint baselen;\n-\tstruct strbuf log;\n+\tint oldlen = name->len;\n \n \tif (!d)\n-\t\treturn *base ? errno : 0;\n-\n-\tbaselen = strlen(base);\n-\tstrbuf_init(&log, baselen + 257);\n-\tstrbuf_add(&log, base, baselen);\n-\tif (log.len && log.buf[log.len-1] != '/') {\n-\t\tstrbuf_addch(&log, '/');\n-\t\tbaselen++;\n-\t}\n+\t\treturn name->len ? errno : 0;\n \n \twhile ((de = readdir(d)) != NULL) {\n \t\tstruct stat st;\n@@ -2268,32 +2264,37 @@ static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)\n \t\t\tcontinue;\n \t\tif (has_extension(de->d_name, \".lock\"))\n \t\t\tcontinue;\n-\t\tstrbuf_addstr(&log, de->d_name);\n-\t\tif (stat(git_path(\"logs/%s\", log.buf), &st) < 0) {\n+\t\tstrbuf_addstr(name, de->d_name);\n+\t\tif (stat(git_path(\"logs/%s\", name->buf), &st) < 0) {\n \t\t\t/* Silently ignore. */\n \t\t} else {\n \t\t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\t\tretval = do_for_each_reflog(log.buf, fn, cb_data);\n+\t\t\t\tstrbuf_addch(name, '/');\n+\t\t\t\tretval = do_for_each_reflog(name, fn, cb_data);\n \t\t\t} else {\n \t\t\t\tunsigned char sha1[20];\n-\t\t\t\tif (read_ref_full(log.buf, sha1, 0, NULL))\n-\t\t\t\t\tretval = error(\"bad ref for %s\", log.buf);\n+\t\t\t\tif (read_ref_full(name->buf, sha1, 0, NULL))\n+\t\t\t\t\tretval = error(\"bad ref for %s\", name->buf);\n \t\t\t\telse\n-\t\t\t\t\tretval = fn(log.buf, sha1, 0, cb_data);\n+\t\t\t\t\tretval = fn(name->buf, sha1, 0, cb_data);\n \t\t\t}\n \t\t\tif (retval)\n \t\t\t\tbreak;\n \t\t}\n-\t\tstrbuf_setlen(&log, baselen);\n+\t\tstrbuf_setlen(name, oldlen);\n \t}\n-\tstrbuf_release(&log);\n \tclosedir(d);\n \treturn retval;\n }\n \n int for_each_reflog(each_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_reflog(\"\", fn, cb_data);\n+\tint retval;\n+\tstruct strbuf name;\n+\tstrbuf_init(&name, PATH_MAX);\n+\tretval = do_for_each_reflog(&name, fn, cb_data);\n+\tstrbuf_release(&name);\n+\treturn retval;\n }\n \n int update_ref(const char *action, const char *refname,\n-- \n1.7.10\n"},{"id":"190183","messageId":"1335479227-7877-11-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 10/18] bisect: copy filename string obtained from git_path()","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:26:59Z","receivedAt":"2012-04-26T22:26:59Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nPrevent the string from being overwritten by other callers of\ngit_path() and friends before we are done using it.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n\nOtherwise, this bug will be triggered by later patches in the series.\n\nI didn't do a careful code audit of this problem, but it seems very\nplausible that that check_ancestors() and/or check_merge_bases() are\nguilty.  They certainly do a lot more than should be done while\nholding on to a pointer to a statically-allocated buffer.\n\nI cursorily checked other code in the neighborhood for similar abuses,\nbut it would be good for an expert to look it over.\n\n bisect.c |    8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 6e186e2..48acf73 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -833,7 +833,7 @@ static int check_ancestors(const char *prefix)\n  */\n static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)\n {\n-\tconst char *filename = git_path(\"BISECT_ANCESTORS_OK\");\n+\tchar *filename = xstrdup(git_path(\"BISECT_ANCESTORS_OK\"));\n \tstruct stat st;\n \tint fd;\n \n@@ -842,11 +842,11 @@ static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)\n \n \t/* Check if file BISECT_ANCESTORS_OK exists. */\n \tif (!stat(filename, &st) && S_ISREG(st.st_mode))\n-\t\treturn;\n+\t\tgoto done;\n \n \t/* Bisecting with no good rev is ok. */\n \tif (good_revs.nr == 0)\n-\t\treturn;\n+\t\tgoto done;\n \n \t/* Check if all good revs are ancestor of the bad rev. */\n \tif (check_ancestors(prefix))\n@@ -859,6 +859,8 @@ static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)\n \t\t\tfilename, strerror(errno));\n \telse\n \t\tclose(fd);\n+ done:\n+\tfree(filename);\n }\n \n /*\n-- \n1.7.10\n"},{"id":"190171","messageId":"1335479227-7877-12-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 11/18] find_containing_dir(): use strbuf in implementation of this function","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:27:00Z","receivedAt":"2012-04-26T22:27:00Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   19 ++++++++++---------\n 1 file changed, 10 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 989c10d..94b20a3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -309,20 +309,21 @@ static struct ref_entry *search_for_subdir(struct ref_dir *dir,\n static struct ref_dir *find_containing_dir(struct ref_dir *dir,\n \t\t\t\t\t   const char *refname, int mkdir)\n {\n-\tchar *refname_copy = xstrdup(refname);\n-\tchar *slash;\n-\tstruct ref_entry *entry;\n-\tfor (slash = strchr(refname_copy, '/'); slash; slash = strchr(slash + 1, '/')) {\n-\t\tchar tmp = slash[1];\n-\t\tslash[1] = '\\0';\n-\t\tentry = search_for_subdir(dir, refname_copy, mkdir);\n-\t\tslash[1] = tmp;\n+\tstruct strbuf dirname;\n+\tconst char *slash;\n+\tstrbuf_init(&dirname, PATH_MAX);\n+\tfor (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {\n+\t\tstruct ref_entry *entry;\n+\t\tstrbuf_add(&dirname,\n+\t\t\t   refname + dirname.len,\n+\t\t\t   (slash + 1) - (refname + dirname.len));\n+\t\tentry = search_for_subdir(dir, dirname.buf, mkdir);\n \t\tif (!entry)\n \t\t\tbreak;\n \t\tdir = &entry->u.subdir;\n \t}\n \n-\tfree(refname_copy);\n+\tstrbuf_release(&dirname);\n \treturn dir;\n }\n \n-- \n1.7.10\n"},{"id":"190173","messageId":"1335479227-7877-13-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 12/18] refs: wrap top-level ref_dirs in ref_entries","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:27:01Z","receivedAt":"2012-04-26T22:27:01Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nMake it turtles all the way down.  This affects the loose and packed\nfields of ref_cache instances.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n\nThis change is not crucial, as it was in v1 of the patch series.\nNevertheless I think that it improves the code consistency.\n\n refs.c |   37 +++++++++++++++++++------------------\n 1 file changed, 19 insertions(+), 18 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 94b20a3..de14e75 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -607,26 +607,26 @@ static int is_refname_available(const char *refname, const char *oldrefname,\n  */\n static struct ref_cache {\n \tstruct ref_cache *next;\n-\tchar did_loose;\n-\tchar did_packed;\n-\tstruct ref_dir loose;\n-\tstruct ref_dir packed;\n+\tstruct ref_entry *loose;\n+\tstruct ref_entry *packed;\n \t/* The submodule name, or \"\" for the main repo. */\n \tchar name[FLEX_ARRAY];\n } *ref_cache;\n \n static void clear_packed_ref_cache(struct ref_cache *refs)\n {\n-\tif (refs->did_packed)\n-\t\tclear_ref_dir(&refs->packed);\n-\trefs->did_packed = 0;\n+\tif (refs->packed) {\n+\t\tfree_ref_entry(refs->packed);\n+\t\trefs->packed = NULL;\n+\t}\n }\n \n static void clear_loose_ref_cache(struct ref_cache *refs)\n {\n-\tif (refs->did_loose)\n-\t\tclear_ref_dir(&refs->loose);\n-\trefs->did_loose = 0;\n+\tif (refs->loose) {\n+\t\tfree_ref_entry(refs->loose);\n+\t\trefs->loose = NULL;\n+\t}\n }\n \n static struct ref_cache *create_ref_cache(const char *submodule)\n@@ -740,22 +740,22 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)\n \n static struct ref_dir *get_packed_refs(struct ref_cache *refs)\n {\n-\tif (!refs->did_packed) {\n+\tif (!refs->packed) {\n \t\tconst char *packed_refs_file;\n \t\tFILE *f;\n \n+\t\trefs->packed = create_dir_entry(\"\");\n \t\tif (*refs->name)\n \t\t\tpacked_refs_file = git_path_submodule(refs->name, \"packed-refs\");\n \t\telse\n \t\t\tpacked_refs_file = git_path(\"packed-refs\");\n \t\tf = fopen(packed_refs_file, \"r\");\n \t\tif (f) {\n-\t\t\tread_packed_refs(f, &refs->packed);\n+\t\t\tread_packed_refs(f, &refs->packed->u.subdir);\n \t\t\tfclose(f);\n \t\t}\n-\t\trefs->did_packed = 1;\n \t}\n-\treturn &refs->packed;\n+\treturn &refs->packed->u.subdir;\n }\n \n void add_packed_ref(const char *refname, const unsigned char *sha1)\n@@ -833,12 +833,13 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,\n \n static struct ref_dir *get_loose_refs(struct ref_cache *refs)\n {\n-\tif (!refs->did_loose) {\n+\tif (!refs->loose) {\n+\t\trefs->loose = create_dir_entry(\"\");\n \t\tget_ref_dir(refs, \"refs/\",\n-\t\t\t    &search_for_subdir(&refs->loose, \"refs/\", 1)->u.subdir);\n-\t\trefs->did_loose = 1;\n+\t\t\t    &search_for_subdir(&refs->loose->u.subdir,\n+\t\t\t\t\t       \"refs/\", 1)->u.subdir);\n \t}\n-\treturn &refs->loose;\n+\treturn &refs->loose->u.subdir;\n }\n \n /* We allow \"recursive\" symbolic refs. Only within reason, though */\n-- \n1.7.10\n"},{"id":"190174","messageId":"1335479227-7877-14-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 13/18] read_loose_refs(): rename function from get_ref_dir()","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:27:02Z","receivedAt":"2012-04-26T22:27:02Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThe new name better describes the function's purpose, and also makes\nthe old name available for a more suitable purpose.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex de14e75..1d18ae6 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -769,8 +769,8 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)\n  * dirname must end with '/'.  dir must be the directory entry\n  * corresponding to dirname.\n  */\n-static void get_ref_dir(struct ref_cache *refs, const char *dirname,\n-\t\t\tstruct ref_dir *dir)\n+static void read_loose_refs(struct ref_cache *refs, const char *dirname,\n+\t\t\t    struct ref_dir *dir)\n {\n \tDIR *d;\n \tconst char *path;\n@@ -808,8 +808,8 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,\n \t\t\t/* Silently ignore. */\n \t\t} else if (S_ISDIR(st.st_mode)) {\n \t\t\tstrbuf_addch(&refname, '/');\n-\t\t\tget_ref_dir(refs, refname.buf,\n-\t\t\t\t    &search_for_subdir(dir, refname.buf, 1)->u.subdir);\n+\t\t\tread_loose_refs(refs, refname.buf,\n+\t\t\t\t\t&search_for_subdir(dir, refname.buf, 1)->u.subdir);\n \t\t} else {\n \t\t\tif (*refs->name) {\n \t\t\t\thashclr(sha1);\n@@ -835,9 +835,9 @@ static struct ref_dir *get_loose_refs(struct ref_cache *refs)\n {\n \tif (!refs->loose) {\n \t\trefs->loose = create_dir_entry(\"\");\n-\t\tget_ref_dir(refs, \"refs/\",\n-\t\t\t    &search_for_subdir(&refs->loose->u.subdir,\n-\t\t\t\t\t       \"refs/\", 1)->u.subdir);\n+\t\tread_loose_refs(refs, \"refs/\",\n+\t\t\t\t&search_for_subdir(&refs->loose->u.subdir,\n+\t\t\t\t\t\t   \"refs/\", 1)->u.subdir);\n \t}\n \treturn &refs->loose->u.subdir;\n }\n-- \n1.7.10\n"},{"id":"190176","messageId":"1335479227-7877-15-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 14/18] get_ref_dir(): add function for getting a ref_dir from a ref_entry","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:27:03Z","receivedAt":"2012-04-26T22:27:03Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nConvert all accesses of a ref_dir within a ref_entry to use this\nfunction.  This function will later be responsible for reading loose\nreferences from disk on demand.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   40 +++++++++++++++++++++++++---------------\n 1 file changed, 25 insertions(+), 15 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 1d18ae6..2e71bfa 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -171,6 +171,12 @@ struct ref_entry {\n \tchar name[FLEX_ARRAY];\n };\n \n+static struct ref_dir *get_ref_dir(struct ref_entry *entry)\n+{\n+\tassert(entry->flag & REF_DIR);\n+\treturn &entry->u.subdir;\n+}\n+\n static struct ref_entry *create_ref_entry(const char *refname,\n \t\t\t\t\t  const unsigned char *sha1, int flag,\n \t\t\t\t\t  int check_name)\n@@ -195,7 +201,7 @@ static void clear_ref_dir(struct ref_dir *dir);\n static void free_ref_entry(struct ref_entry *entry)\n {\n \tif (entry->flag & REF_DIR)\n-\t\tclear_ref_dir(&entry->u.subdir);\n+\t\tclear_ref_dir(get_ref_dir(entry));\n \tfree(entry);\n }\n \n@@ -320,7 +326,7 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,\n \t\tentry = search_for_subdir(dir, dirname.buf, mkdir);\n \t\tif (!entry)\n \t\t\tbreak;\n-\t\tdir = &entry->u.subdir;\n+\t\tdir = get_ref_dir(entry);\n \t}\n \n \tstrbuf_release(&dirname);\n@@ -449,8 +455,9 @@ static int do_for_each_ref_in_dir(struct ref_dir *dir, int offset,\n \t\tstruct ref_entry *entry = dir->entries[i];\n \t\tint retval;\n \t\tif (entry->flag & REF_DIR) {\n-\t\t\tsort_ref_dir(&entry->u.subdir);\n-\t\t\tretval = do_for_each_ref_in_dir(&entry->u.subdir, 0,\n+\t\t\tstruct ref_dir *subdir = get_ref_dir(entry);\n+\t\t\tsort_ref_dir(subdir);\n+\t\t\tretval = do_for_each_ref_in_dir(subdir, 0,\n \t\t\t\t\t\t\tbase, fn, trim, flags, cb_data);\n \t\t} else {\n \t\t\tretval = do_one_ref(base, fn, trim, flags, cb_data, entry);\n@@ -495,10 +502,12 @@ static int do_for_each_ref_in_dirs(struct ref_dir *dir1,\n \t\tif (cmp == 0) {\n \t\t\tif ((e1->flag & REF_DIR) && (e2->flag & REF_DIR)) {\n \t\t\t\t/* Both are directories; descend them in parallel. */\n-\t\t\t\tsort_ref_dir(&e1->u.subdir);\n-\t\t\t\tsort_ref_dir(&e2->u.subdir);\n+\t\t\t\tstruct ref_dir *subdir1 = get_ref_dir(e1);\n+\t\t\t\tstruct ref_dir *subdir2 = get_ref_dir(e2);\n+\t\t\t\tsort_ref_dir(subdir1);\n+\t\t\t\tsort_ref_dir(subdir2);\n \t\t\t\tretval = do_for_each_ref_in_dirs(\n-\t\t\t\t\t\t&e1->u.subdir, &e2->u.subdir,\n+\t\t\t\t\t\tsubdir1, subdir2,\n \t\t\t\t\t\tbase, fn, trim, flags, cb_data);\n \t\t\t\ti1++;\n \t\t\t\ti2++;\n@@ -521,9 +530,10 @@ static int do_for_each_ref_in_dirs(struct ref_dir *dir1,\n \t\t\t\ti2++;\n \t\t\t}\n \t\t\tif (e->flag & REF_DIR) {\n-\t\t\t\tsort_ref_dir(&e->u.subdir);\n+\t\t\t\tstruct ref_dir *subdir = get_ref_dir(e);\n+\t\t\t\tsort_ref_dir(subdir);\n \t\t\t\tretval = do_for_each_ref_in_dir(\n-\t\t\t\t\t\t&e->u.subdir, 0,\n+\t\t\t\t\t\tsubdir, 0,\n \t\t\t\t\t\tbase, fn, trim, flags, cb_data);\n \t\t\t} else {\n \t\t\t\tretval = do_one_ref(base, fn, trim, flags, cb_data, e);\n@@ -751,11 +761,11 @@ static struct ref_dir *get_packed_refs(struct ref_cache *refs)\n \t\t\tpacked_refs_file = git_path(\"packed-refs\");\n \t\tf = fopen(packed_refs_file, \"r\");\n \t\tif (f) {\n-\t\t\tread_packed_refs(f, &refs->packed->u.subdir);\n+\t\t\tread_packed_refs(f, get_ref_dir(refs->packed));\n \t\t\tfclose(f);\n \t\t}\n \t}\n-\treturn &refs->packed->u.subdir;\n+\treturn get_ref_dir(refs->packed);\n }\n \n void add_packed_ref(const char *refname, const unsigned char *sha1)\n@@ -809,7 +819,7 @@ static void read_loose_refs(struct ref_cache *refs, const char *dirname,\n \t\t} else if (S_ISDIR(st.st_mode)) {\n \t\t\tstrbuf_addch(&refname, '/');\n \t\t\tread_loose_refs(refs, refname.buf,\n-\t\t\t\t\t&search_for_subdir(dir, refname.buf, 1)->u.subdir);\n+\t\t\t\t\tget_ref_dir(search_for_subdir(dir, refname.buf, 1)));\n \t\t} else {\n \t\t\tif (*refs->name) {\n \t\t\t\thashclr(sha1);\n@@ -836,10 +846,10 @@ static struct ref_dir *get_loose_refs(struct ref_cache *refs)\n \tif (!refs->loose) {\n \t\trefs->loose = create_dir_entry(\"\");\n \t\tread_loose_refs(refs, \"refs/\",\n-\t\t\t\t&search_for_subdir(&refs->loose->u.subdir,\n-\t\t\t\t\t\t   \"refs/\", 1)->u.subdir);\n+\t\t\t\tget_ref_dir(search_for_subdir(get_ref_dir(refs->loose),\n+\t\t\t\t\t\t\t      \"refs/\", 1)));\n \t}\n-\treturn &refs->loose->u.subdir;\n+\treturn get_ref_dir(refs->loose);\n }\n \n /* We allow \"recursive\" symbolic refs. Only within reason, though */\n-- \n1.7.10\n"},{"id":"190175","messageId":"1335479227-7877-16-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 15/18] search_for_subdir(): return (ref_dir *) instead of (ref_entry *)","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:27:04Z","receivedAt":"2012-04-26T22:27:04Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThat is what all the callers want, so give it to them.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   21 ++++++++++-----------\n 1 file changed, 10 insertions(+), 11 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 2e71bfa..f6447e3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -289,8 +289,8 @@ static struct ref_entry *search_ref_dir(struct ref_dir *dir, const char *refname\n  * directory if it is missing; otherwise, return NULL if the desired\n  * directory cannot be found.\n  */\n-static struct ref_entry *search_for_subdir(struct ref_dir *dir,\n-\t\t\t\t\t   const char *subdirname, int mkdir)\n+static struct ref_dir *search_for_subdir(struct ref_dir *dir,\n+\t\t\t\t\t const char *subdirname, int mkdir)\n {\n \tstruct ref_entry *entry = search_ref_dir(dir, subdirname);\n \tif (!entry) {\n@@ -299,8 +299,7 @@ static struct ref_entry *search_for_subdir(struct ref_dir *dir,\n \t\tentry = create_dir_entry(subdirname);\n \t\tadd_entry_to_dir(dir, entry);\n \t}\n-\tassert(entry->flag & REF_DIR);\n-\treturn entry;\n+\treturn get_ref_dir(entry);\n }\n \n /*\n@@ -319,14 +318,14 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,\n \tconst char *slash;\n \tstrbuf_init(&dirname, PATH_MAX);\n \tfor (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {\n-\t\tstruct ref_entry *entry;\n+\t\tstruct ref_dir *subdir;\n \t\tstrbuf_add(&dirname,\n \t\t\t   refname + dirname.len,\n \t\t\t   (slash + 1) - (refname + dirname.len));\n-\t\tentry = search_for_subdir(dir, dirname.buf, mkdir);\n-\t\tif (!entry)\n+\t\tsubdir = search_for_subdir(dir, dirname.buf, mkdir);\n+\t\tif (!subdir)\n \t\t\tbreak;\n-\t\tdir = get_ref_dir(entry);\n+\t\tdir = subdir;\n \t}\n \n \tstrbuf_release(&dirname);\n@@ -819,7 +818,7 @@ static void read_loose_refs(struct ref_cache *refs, const char *dirname,\n \t\t} else if (S_ISDIR(st.st_mode)) {\n \t\t\tstrbuf_addch(&refname, '/');\n \t\t\tread_loose_refs(refs, refname.buf,\n-\t\t\t\t\tget_ref_dir(search_for_subdir(dir, refname.buf, 1)));\n+\t\t\t\t\tsearch_for_subdir(dir, refname.buf, 1));\n \t\t} else {\n \t\t\tif (*refs->name) {\n \t\t\t\thashclr(sha1);\n@@ -846,8 +845,8 @@ static struct ref_dir *get_loose_refs(struct ref_cache *refs)\n \tif (!refs->loose) {\n \t\trefs->loose = create_dir_entry(\"\");\n \t\tread_loose_refs(refs, \"refs/\",\n-\t\t\t\tget_ref_dir(search_for_subdir(get_ref_dir(refs->loose),\n-\t\t\t\t\t\t\t      \"refs/\", 1)));\n+\t\t\t\tsearch_for_subdir(get_ref_dir(refs->loose),\n+\t\t\t\t\t\t  \"refs/\", 1));\n \t}\n \treturn get_ref_dir(refs->loose);\n }\n-- \n1.7.10\n"},{"id":"190179","messageId":"1335479227-7877-17-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 16/18] struct ref_dir: store a reference to the enclosing ref_cache","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:27:05Z","receivedAt":"2012-04-26T22:27:05Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThis means that a directory ref_entry contains all of the information\nneeded by read_loose_refs().\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |   15 +++++++++++----\n 1 file changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex f6447e3..110e801 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -106,6 +106,8 @@ struct ref_value {\n \tunsigned char peeled[20];\n };\n \n+struct ref_cache;\n+\n struct ref_dir {\n \tint nr, alloc;\n \n@@ -117,6 +119,9 @@ struct ref_dir {\n \t */\n \tint sorted;\n \n+\t/* A pointer to the ref_cache that contains this ref_dir. */\n+\tstruct ref_cache *ref_cache;\n+\n \tstruct ref_entry **entries;\n };\n \n@@ -234,12 +239,14 @@ static void clear_ref_dir(struct ref_dir *dir)\n  * dirname is the name of the directory with a trailing slash (e.g.,\n  * \"refs/heads/\") or \"\" for the top-level directory.\n  */\n-static struct ref_entry *create_dir_entry(const char *dirname)\n+static struct ref_entry *create_dir_entry(struct ref_cache *ref_cache,\n+\t\t\t\t\t  const char *dirname)\n {\n \tstruct ref_entry *direntry;\n \tint len = strlen(dirname);\n \tdirentry = xcalloc(1, sizeof(struct ref_entry) + len + 1);\n \tmemcpy(direntry->name, dirname, len + 1);\n+\tdirentry->u.subdir.ref_cache = ref_cache;\n \tdirentry->flag = REF_DIR;\n \treturn direntry;\n }\n@@ -296,7 +303,7 @@ static struct ref_dir *search_for_subdir(struct ref_dir *dir,\n \tif (!entry) {\n \t\tif (!mkdir)\n \t\t\treturn NULL;\n-\t\tentry = create_dir_entry(subdirname);\n+\t\tentry = create_dir_entry(dir->ref_cache, subdirname);\n \t\tadd_entry_to_dir(dir, entry);\n \t}\n \treturn get_ref_dir(entry);\n@@ -753,7 +760,7 @@ static struct ref_dir *get_packed_refs(struct ref_cache *refs)\n \t\tconst char *packed_refs_file;\n \t\tFILE *f;\n \n-\t\trefs->packed = create_dir_entry(\"\");\n+\t\trefs->packed = create_dir_entry(refs, \"\");\n \t\tif (*refs->name)\n \t\t\tpacked_refs_file = git_path_submodule(refs->name, \"packed-refs\");\n \t\telse\n@@ -843,7 +850,7 @@ static void read_loose_refs(struct ref_cache *refs, const char *dirname,\n static struct ref_dir *get_loose_refs(struct ref_cache *refs)\n {\n \tif (!refs->loose) {\n-\t\trefs->loose = create_dir_entry(\"\");\n+\t\trefs->loose = create_dir_entry(refs, \"\");\n \t\tread_loose_refs(refs, \"refs/\",\n \t\t\t\tsearch_for_subdir(get_ref_dir(refs->loose),\n \t\t\t\t\t\t  \"refs/\", 1));\n-- \n1.7.10\n"},{"id":"190177","messageId":"1335479227-7877-18-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 17/18] read_loose_refs(): eliminate ref_cache argument","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:27:06Z","receivedAt":"2012-04-26T22:27:06Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nThe ref_cache can now be read from the ref_dir.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |    8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 110e801..a0c687f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -785,9 +785,9 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)\n  * dirname must end with '/'.  dir must be the directory entry\n  * corresponding to dirname.\n  */\n-static void read_loose_refs(struct ref_cache *refs, const char *dirname,\n-\t\t\t    struct ref_dir *dir)\n+static void read_loose_refs(const char *dirname, struct ref_dir *dir)\n {\n+\tstruct ref_cache *refs = dir->ref_cache;\n \tDIR *d;\n \tconst char *path;\n \tstruct dirent *de;\n@@ -824,7 +824,7 @@ static void read_loose_refs(struct ref_cache *refs, const char *dirname,\n \t\t\t/* Silently ignore. */\n \t\t} else if (S_ISDIR(st.st_mode)) {\n \t\t\tstrbuf_addch(&refname, '/');\n-\t\t\tread_loose_refs(refs, refname.buf,\n+\t\t\tread_loose_refs(refname.buf,\n \t\t\t\t\tsearch_for_subdir(dir, refname.buf, 1));\n \t\t} else {\n \t\t\tif (*refs->name) {\n@@ -851,7 +851,7 @@ static struct ref_dir *get_loose_refs(struct ref_cache *refs)\n {\n \tif (!refs->loose) {\n \t\trefs->loose = create_dir_entry(refs, \"\");\n-\t\tread_loose_refs(refs, \"refs/\",\n+\t\tread_loose_refs(\"refs/\",\n \t\t\t\tsearch_for_subdir(get_ref_dir(refs->loose),\n \t\t\t\t\t\t  \"refs/\", 1));\n \t}\n-- \n1.7.10\n"},{"id":"190178","messageId":"1335479227-7877-19-git-send-email-mhagger@alum.mit.edu","threadId":"30345","inReplyTo":"1335479227-7877-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 18/18] refs: read loose references lazily","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-26T22:27:07Z","receivedAt":"2012-04-26T22:27:07Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nInstead of reading the whole directory of loose references the first\ntime any are needed, only read them on demand, one directory at a\ntime.\n\nUse a new ref_entry flag bit REF_INCOMPLETE to indicate that the entry\nrepresents a REF_DIR that hasn't been read yet.  Whenever any entries\nfrom such a directory are needed, read all of the loose references\nfrom that directory.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c |  127 +++++++++++++++++++++++++++++++++++++++++++++++++---------------\n 1 file changed, 97 insertions(+), 30 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex a0c687f..27ce968 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -101,6 +101,12 @@ int check_refname_format(const char *refname, int flags)\n \n struct ref_entry;\n \n+/*\n+ * Information used (along with the information in ref_entry) to\n+ * describe a single cached reference.  This data structure only\n+ * occurs embedded in a union in struct ref_entry, and only when\n+ * (ref_entry->flag & REF_DIR) is zero.\n+ */\n struct ref_value {\n \tunsigned char sha1[20];\n \tunsigned char peeled[20];\n@@ -108,6 +114,32 @@ struct ref_value {\n \n struct ref_cache;\n \n+/*\n+ * Information used (along with the information in ref_entry) to\n+ * describe a level in the hierarchy of references.  This data\n+ * structure only occurs embedded in a union in struct ref_entry, and\n+ * only when (ref_entry.flag & REF_DIR) is set.  In that case,\n+ * (ref_entry.flag & REF_INCOMPLETE) determines whether the references\n+ * in the directory have already been read:\n+ *\n+ *     (ref_entry.flag & REF_INCOMPLETE) unset -- a directory of loose\n+ *         or packed references, already read.\n+ *\n+ *     (ref_entry.flag & REF_INCOMPLETE) set -- a directory of loose\n+ *         references that hasn't been read yet (nor has any of its\n+ *         subdirectories).\n+ *\n+ * Entries within a directory are stored within a growable array of\n+ * pointers to ref_entries (entries, nr, alloc).  Entries 0 <= i <\n+ * sorted are sorted by their component name in strcmp() order and the\n+ * remaining entries are unsorted.\n+ *\n+ * Loose references are read lazily, one directory at a time.  When a\n+ * directory of loose references is read, then all of the references\n+ * in that directory are stored, and REF_INCOMPLETE stubs are created\n+ * for any subdirectories, but the subdirectories themselves are not\n+ * read.  The reading is triggered by get_ref_dir().\n+ */\n struct ref_dir {\n \tint nr, alloc;\n \n@@ -127,19 +159,33 @@ struct ref_dir {\n \n /* ISSYMREF=0x01, ISPACKED=0x02, and ISBROKEN=0x04 are public interfaces */\n #define REF_KNOWS_PEELED 0x08\n+\n+/* ref_entry represents a directory of references */\n #define REF_DIR 0x10\n \n /*\n+ * Entry has not yet been read from disk (used only for REF_DIR\n+ * entries representing loose references)\n+ */\n+#define REF_INCOMPLETE 0x20\n+\n+/*\n  * A ref_entry represents either a reference or a \"subdirectory\" of\n- * references.  Each directory in the reference namespace is\n- * represented by a ref_entry with (flags & REF_DIR) set and\n- * containing a subdir member that holds the entries in that\n- * directory.  References are represented by a ref_entry with (flags &\n- * REF_DIR) unset and a value member that describes the reference's\n- * value.  The flag member is at the ref_entry level, but it is also\n- * needed to interpret the contents of the value field (in other\n- * words, a ref_value object is not very much use without the\n- * enclosing ref_entry).\n+ * references.\n+ *\n+ * Each directory in the reference namespace is represented by a\n+ * ref_entry with (flags & REF_DIR) set and containing a subdir member\n+ * that holds the entries in that directory that have been read so\n+ * far.  If (flags & REF_INCOMPLETE) is set, then the directory and\n+ * its subdirectories haven't been read yet.  REF_INCOMPLETE is only\n+ * used for loose reference directories.\n+ *\n+ * References are represented by a ref_entry with (flags & REF_DIR)\n+ * unset and a value member that describes the reference's value.  The\n+ * flag member is at the ref_entry level, but it is also needed to\n+ * interpret the contents of the value field (in other words, a\n+ * ref_value object is not very much use without the enclosing\n+ * ref_entry).\n  *\n  * Reference names cannot end with slash and directories' names are\n  * always stored with a trailing slash (except for the top-level\n@@ -176,10 +222,18 @@ struct ref_entry {\n \tchar name[FLEX_ARRAY];\n };\n \n+static void read_loose_refs(const char *dirname, struct ref_dir *dir);\n+\n static struct ref_dir *get_ref_dir(struct ref_entry *entry)\n {\n+\tstruct ref_dir *dir;\n \tassert(entry->flag & REF_DIR);\n-\treturn &entry->u.subdir;\n+\tdir = &entry->u.subdir;\n+\tif (entry->flag & REF_INCOMPLETE) {\n+\t\tread_loose_refs(entry->name, dir);\n+\t\tentry->flag &= ~REF_INCOMPLETE;\n+\t}\n+\treturn dir;\n }\n \n static struct ref_entry *create_ref_entry(const char *refname,\n@@ -237,17 +291,17 @@ static void clear_ref_dir(struct ref_dir *dir)\n /*\n  * Create a struct ref_entry object for the specified dirname.\n  * dirname is the name of the directory with a trailing slash (e.g.,\n- * \"refs/heads/\") or \"\" for the top-level directory.\n+ * \"refs/heads/\") or \"\" for the top-level directory.  \n  */\n static struct ref_entry *create_dir_entry(struct ref_cache *ref_cache,\n-\t\t\t\t\t  const char *dirname)\n+\t\t\t\t\t  const char *dirname, int incomplete)\n {\n \tstruct ref_entry *direntry;\n \tint len = strlen(dirname);\n \tdirentry = xcalloc(1, sizeof(struct ref_entry) + len + 1);\n \tmemcpy(direntry->name, dirname, len + 1);\n \tdirentry->u.subdir.ref_cache = ref_cache;\n-\tdirentry->flag = REF_DIR;\n+\tdirentry->flag = REF_DIR | (incomplete ? REF_INCOMPLETE : 0);\n \treturn direntry;\n }\n \n@@ -263,7 +317,7 @@ static void sort_ref_dir(struct ref_dir *dir);\n /*\n  * Return the entry with the given refname from the ref_dir\n  * (non-recursively), sorting dir if necessary.  Return NULL if no\n- * such entry is found.\n+ * such entry is found.  dir must already be complete.\n  */\n static struct ref_entry *search_ref_dir(struct ref_dir *dir, const char *refname)\n {\n@@ -294,7 +348,7 @@ static struct ref_entry *search_ref_dir(struct ref_dir *dir, const char *refname\n  * recursing).  Sort dir if necessary.  subdirname must be a directory\n  * name (i.e., end in '/').  If mkdir is set, then create the\n  * directory if it is missing; otherwise, return NULL if the desired\n- * directory cannot be found.\n+ * directory cannot be found.  dir must already be complete.\n  */\n static struct ref_dir *search_for_subdir(struct ref_dir *dir,\n \t\t\t\t\t const char *subdirname, int mkdir)\n@@ -303,7 +357,13 @@ static struct ref_dir *search_for_subdir(struct ref_dir *dir,\n \tif (!entry) {\n \t\tif (!mkdir)\n \t\t\treturn NULL;\n-\t\tentry = create_dir_entry(dir->ref_cache, subdirname);\n+\t\t/*\n+\t\t * Since dir is complete, the absence of a subdir\n+\t\t * means that the subdir really doesn't exist;\n+\t\t * therefore, create an empty record for it but mark\n+\t\t * the record complete.\n+\t\t */\n+\t\tentry = create_dir_entry(dir->ref_cache, subdirname, 0);\n \t\tadd_entry_to_dir(dir, entry);\n \t}\n \treturn get_ref_dir(entry);\n@@ -313,10 +373,10 @@ static struct ref_dir *search_for_subdir(struct ref_dir *dir,\n  * If refname is a reference name, find the ref_dir within the dir\n  * tree that should hold refname.  If refname is a directory name\n  * (i.e., ends in '/'), then return that ref_dir itself.  dir must\n- * represent the top-level directory.  Sort ref_dirs and recurse into\n- * subdirectories as necessary.  If mkdir is set, then create any\n- * missing directories; otherwise, return NULL if the desired\n- * directory cannot be found.\n+ * represent the top-level directory and must already be complete.\n+ * Sort ref_dirs and recurse into subdirectories as necessary.  If\n+ * mkdir is set, then create any missing directories; otherwise,\n+ * return NULL if the desired directory cannot be found.\n  */\n static struct ref_dir *find_containing_dir(struct ref_dir *dir,\n \t\t\t\t\t   const char *refname, int mkdir)\n@@ -760,7 +820,7 @@ static struct ref_dir *get_packed_refs(struct ref_cache *refs)\n \t\tconst char *packed_refs_file;\n \t\tFILE *f;\n \n-\t\trefs->packed = create_dir_entry(refs, \"\");\n+\t\trefs->packed = create_dir_entry(refs, \"\", 0);\n \t\tif (*refs->name)\n \t\t\tpacked_refs_file = git_path_submodule(refs->name, \"packed-refs\");\n \t\telse\n@@ -781,9 +841,9 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)\n }\n \n /*\n- * Read the loose references for refs from the namespace dirname.\n- * dirname must end with '/'.  dir must be the directory entry\n- * corresponding to dirname.\n+ * Read the loose references from the namespace dirname into dir\n+ * (without recursing).  dirname must end with '/'.  dir must be the\n+ * directory entry corresponding to dirname.\n  */\n static void read_loose_refs(const char *dirname, struct ref_dir *dir)\n {\n@@ -824,8 +884,8 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)\n \t\t\t/* Silently ignore. */\n \t\t} else if (S_ISDIR(st.st_mode)) {\n \t\t\tstrbuf_addch(&refname, '/');\n-\t\t\tread_loose_refs(refname.buf,\n-\t\t\t\t\tsearch_for_subdir(dir, refname.buf, 1));\n+\t\t\tadd_entry_to_dir(dir,\n+\t\t\t\t\t create_dir_entry(refs, refname.buf, 1));\n \t\t} else {\n \t\t\tif (*refs->name) {\n \t\t\t\thashclr(sha1);\n@@ -850,10 +910,17 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)\n static struct ref_dir *get_loose_refs(struct ref_cache *refs)\n {\n \tif (!refs->loose) {\n-\t\trefs->loose = create_dir_entry(refs, \"\");\n-\t\tread_loose_refs(\"refs/\",\n-\t\t\t\tsearch_for_subdir(get_ref_dir(refs->loose),\n-\t\t\t\t\t\t  \"refs/\", 1));\n+\t\t/*\n+\t\t * Mark the top-level directory complete because we\n+\t\t * are about to read the only subdirectory that can\n+\t\t * hold references:\n+\t\t */\n+\t\trefs->loose = create_dir_entry(refs, \"\", 0);\n+\t\t/*\n+\t\t * Create an incomplete entry for \"refs/\":\n+\t\t */\n+\t\tadd_entry_to_dir(get_ref_dir(refs->loose),\n+\t\t\t\t create_dir_entry(refs, \"refs/\", 1));\n \t}\n \treturn get_ref_dir(refs->loose);\n }\n-- \n1.7.10\n"},{"id":"190186","messageId":"xmqqaa1y6poq.fsf@junio.mtv.corp.google.com","threadId":"30345","inReplyTo":"1335479227-7877-9-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name","fromName":"Junio C Hamano","fromEmail":"junio@pobox.com","sentAt":"2012-04-26T23:25:41Z","receivedAt":"2012-04-26T23:25:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"mhagger@alum.mit.edu writes:\n\n> From: Michael Haggerty <mhagger@alum.mit.edu>\n>\n> This simplifies the bookkeeping and allows an (artificial) restriction\n> on refname component length to be removed.\n>\n> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n> ---\n>  refs.c |   45 +++++++++++++++++++++++----------------------\n>  1 file changed, 23 insertions(+), 22 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index 1d25151..f43c255 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -2248,44 +2248,45 @@ static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)\n>  \tint retval = 0;\n>  \tstruct dirent *de;\n>  \tint baselen;\n> -\tchar *log;\n> +\tstruct strbuf log;\n>  \n>  \tif (!d)\n>  \t\treturn *base ? errno : 0;\n>  \n>  \tbaselen = strlen(base);\n> -\tlog = xmalloc(baselen + 257);\n> -\tmemcpy(log, base, baselen);\n> -\tif (baselen && base[baselen-1] != '/')\n> -\t\tlog[baselen++] = '/';\n> +\tstrbuf_init(&log, baselen + 257);\n> +\tstrbuf_add(&log, base, baselen);\n> +\tif (log.len && log.buf[log.len-1] != '/') {\n> +\t\tstrbuf_addch(&log, '/');\n> +\t\tbaselen++;\n> +\t}\n>  \n>  \twhile ((de = readdir(d)) != NULL) {\n>  \t\tstruct stat st;\n> -\t\tint namelen;\n>  \n>  \t\tif (de->d_name[0] == '.')\n>  \t\t\tcontinue;\n> -\t\tnamelen = strlen(de->d_name);\n> -\t\tif (namelen > 255)\n> -\t\t\tcontinue;\n>  \t\tif (has_extension(de->d_name, \".lock\"))\n>  \t\t\tcontinue;\n> -\t\tmemcpy(log + baselen, de->d_name, namelen+1);\n> -\t\tif (stat(git_path(\"logs/%s\", log), &st) < 0)\n> -\t\t\tcontinue;\n> -\t\tif (S_ISDIR(st.st_mode)) {\n> -\t\t\tretval = do_for_each_reflog(log, fn, cb_data);\n> +\t\tstrbuf_addstr(&log, de->d_name);\n> +\t\tif (stat(git_path(\"logs/%s\", log.buf), &st) < 0) {\n> +\t\t\t/* Silently ignore. */\n>  \t\t} else {\n\nPlease write this like this:\n\n\t\tif (...) {\n\t\t\t; /* silently ignore */\n\t\t}\n\nto make the \"emptyness\" stand out (I amended the previous round when I\nqueued them to 'pu', but I forgot to point it out in my review message).\n"},{"id":"190200","messageId":"4F9A5FE6.40409@alum.mit.edu","threadId":"30345","inReplyTo":"xmqqaa1y6poq.fsf@junio.mtv.corp.google.com","subject":"Re: [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-04-27T08:59:18Z","receivedAt":"2012-04-27T08:59:18Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 04/27/2012 01:25 AM, Junio C Hamano wrote:\n> Please write this like this:\n>\n> \t\tif (...) {\n> \t\t\t; /* silently ignore */\n> \t\t}\n>\n> to make the \"emptyness\" stand out (I amended the previous round when I\n> queued them to 'pu', but I forgot to point it out in my review message).\n\nOK.  A similar construct is in patch 2 of the same series.  I've fixed \nthem in my repo and will use the fixed versions if there are any future \nre-rolls.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"190594","messageId":"7vwr4ugxel.fsf@alter.siamese.dyndns.org","threadId":"30345","inReplyTo":"4F9A5FE6.40409@alum.mit.edu","subject":"Re: [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-02T20:06:58Z","receivedAt":"2012-05-02T20:06:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> On 04/27/2012 01:25 AM, Junio C Hamano wrote:\n>> Please write this like this:\n>>\n>> \t\tif (...) {\n>> \t\t\t; /* silently ignore */\n>> \t\t}\n>>\n>> to make the \"emptyness\" stand out (I amended the previous round when I\n>> queued them to 'pu', but I forgot to point it out in my review message).\n>\n> OK.  A similar construct is in patch 2 of the same series.  I've fixed\n> them in my repo and will use the fixed versions if there are any\n> future re-rolls.\n\nOK.  Has there been any re-roll I missed, or is what I have in 'pu' with\nfixups ready for 'next'?\n"},{"id":"190623","messageId":"4FA22A19.1030009@alum.mit.edu","threadId":"30345","inReplyTo":"7vwr4ugxel.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-05-03T06:47:53Z","receivedAt":"2012-05-03T06:47:53Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/02/2012 10:06 PM, Junio C Hamano wrote:\n> Michael Haggerty<mhagger@alum.mit.edu>  writes:\n>\n>> On 04/27/2012 01:25 AM, Junio C Hamano wrote:\n>>> Please write this like this:\n>>>\n>>> \t\tif (...) {\n>>> \t\t\t; /* silently ignore */\n>>> \t\t}\n>>>\n>>> to make the \"emptyness\" stand out (I amended the previous round when I\n>>> queued them to 'pu', but I forgot to point it out in my review message).\n>>\n>> OK.  A similar construct is in patch 2 of the same series.  I've fixed\n>> them in my repo and will use the fixed versions if there are any\n>> future re-rolls.\n>\n> OK.  Has there been any re-roll I missed, or is what I have in 'pu' with\n> fixups ready for 'next'?\n\nI haven't gotten any other feedback, so as far as I'm concerned, it's \nready for next.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"190681","messageId":"7v4nrxf3m2.fsf@alter.siamese.dyndns.org","threadId":"30345","inReplyTo":"1335479227-7877-6-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 05/18] refs.c: extract function search_for_subdir()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-03T19:48:05Z","receivedAt":"2012-05-03T19:48:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"mhagger@alum.mit.edu writes:\n\n> From: Michael Haggerty <mhagger@alum.mit.edu>\n>\n>\n> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n> ---\n>  refs.c |   34 ++++++++++++++++++++++++----------\n>  1 file changed, 24 insertions(+), 10 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index 01fcdc7..5e51c10 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -277,6 +277,27 @@ static struct ref_entry *search_ref_dir(struct ref_dir *dir, const char *refname\n>  }\n>  \n>  /*\n> + * Search for a directory entry directly within dir (without\n> + * recursing).  Sort dir if necessary.  subdirname must be a directory\n> + * name (i.e., end in '/').  If mkdir is set, then create the\n> + * directory if it is missing; otherwise, return NULL if the desired\n> + * directory cannot be found.\n> + */\n> +static struct ref_entry *search_for_subdir(struct ref_dir *dir,\n> +\t\t\t\t\t   const char *subdirname, int mkdir)\n> +{\n> +\tstruct ref_entry *entry = search_ref_dir(dir, subdirname);\n> +\tif (!entry) {\n> +\t\tif (!mkdir)\n> +\t\t\treturn NULL;\n> +\t\tentry = create_dir_entry(subdirname);\n> +\t\tadd_entry_to_dir(dir, entry);\n> +\t}\n> +\tassert(entry->flag & REF_DIR);\n> +\treturn entry;\n> +}\n> +\n> +/*\n>   * If refname is a reference name, find the ref_dir within the dir\n>   * tree that should hold refname.  If refname is a directory name\n>   * (i.e., ends in '/'), then return that ref_dir itself.  dir must\n> @@ -294,17 +315,10 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,\n>  \tfor (slash = strchr(refname_copy, '/'); slash; slash = strchr(slash + 1, '/')) {\n>  \t\tchar tmp = slash[1];\n>  \t\tslash[1] = '\\0';\n> -\t\tentry = search_ref_dir(dir, refname_copy);\n> -\t\tif (!entry) {\n> -\t\t\tif (!mkdir) {\n> -\t\t\t\tdir = NULL;\n> -\t\t\t\tbreak;\n> -\t\t\t}\n> -\t\t\tentry = create_dir_entry(refname_copy);\n> -\t\t\tadd_entry_to_dir(dir, entry);\n> -\t\t}\n> +\t\tentry = search_for_subdir(dir, refname_copy, mkdir);\n>  \t\tslash[1] = tmp;\n> -\t\tassert(entry->flag & REF_DIR);\n> +\t\tif (!entry)\n> +\t\t\tbreak;\n>  \t\tdir = &entry->u.subdir;\n>  \t}\n\nHrm.  The old code used to reset \"dir\" to NULL before breaking, so the\nentire function used to return NULL.  Now, it calls search_for_subdir(),\nwhich calls search_ref_dir() and gets NULL in entry, and returns NULL.\n\nWouldn't we end up returning the original parameter \"dir\" instead of NULL\nin that case?  Would that make a difference?\n"},{"id":"190688","messageId":"7vmx5pdlv1.fsf@alter.siamese.dyndns.org","threadId":"30345","inReplyTo":"7v4nrxf3m2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 05/18] refs.c: extract function search_for_subdir()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-03T20:56:50Z","receivedAt":"2012-05-03T20:56:50Z","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> Hrm.  The old code used to reset \"dir\" to NULL before breaking, so the\n> entire function used to return NULL.  Now, it calls search_for_subdir(),\n> which calls search_ref_dir() and gets NULL in entry, and returns NULL.\n>\n> Wouldn't we end up returning the original parameter \"dir\" instead of NULL\n> in that case?  Would that make a difference?\n\nIn other words, isn't something like this necessary?\n\nOtherwise, wouldn't do_for_each_ref() called for a non-existing \"refs/\"\nsubhierarchy in \"base\" start from the top-level packed_dir/loose_dir\nreturned from find_containing_dir(), and end up running do_for_each_ref_in_dirs()\nwith both top-level packed_dir/loose_dir and traversing all of them, only\nto find nothing?\n\n refs.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 9f2da16..af5da5f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -390,8 +390,10 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,\n \t\t\t   refname + dirname.len,\n \t\t\t   (slash + 1) - (refname + dirname.len));\n \t\tsubdir = search_for_subdir(dir, dirname.buf, mkdir);\n-\t\tif (!subdir)\n+\t\tif (!subdir) {\n+\t\t\tdir = NULL;\n \t\t\tbreak;\n+\t\t}\n \t\tdir = subdir;\n \t}\n \n"},{"id":"190733","messageId":"4FA3843C.80603@alum.mit.edu","threadId":"30345","inReplyTo":"7vmx5pdlv1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 05/18] refs.c: extract function search_for_subdir()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-05-04T07:24:44Z","receivedAt":"2012-05-04T07:24:44Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/03/2012 10:56 PM, Junio C Hamano wrote:\n> Junio C Hamano<gitster@pobox.com>  writes:\n>\n>> Hrm.  The old code used to reset \"dir\" to NULL before breaking, so the\n>> entire function used to return NULL.  Now, it calls search_for_subdir(),\n>> which calls search_ref_dir() and gets NULL in entry, and returns NULL.\n>>\n>> Wouldn't we end up returning the original parameter \"dir\" instead of NULL\n>> in that case?  Would that make a difference?\n>\n> In other words, isn't something like this necessary?\n\nYou are right.  Thanks for catching this.\n\n> Otherwise, wouldn't do_for_each_ref() called for a non-existing \"refs/\"\n> subhierarchy in \"base\" start from the top-level packed_dir/loose_dir\n> returned from find_containing_dir(), and end up running do_for_each_ref_in_dirs()\n> with both top-level packed_dir/loose_dir and traversing all of them, only\n> to find nothing?\n>\n>   refs.c | 4 +++-\n>   1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/refs.c b/refs.c\n> index 9f2da16..af5da5f 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -390,8 +390,10 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,\n>   \t\t\t   refname + dirname.len,\n>   \t\t\t   (slash + 1) - (refname + dirname.len));\n>   \t\tsubdir = search_for_subdir(dir, dirname.buf, mkdir);\n> -\t\tif (!subdir)\n> +\t\tif (!subdir) {\n> +\t\t\tdir = NULL;\n>   \t\t\tbreak;\n> +\t\t}\n>   \t\tdir = subdir;\n>   \t}\n>\n\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"}]}