* [PATCH v2 01/18] get_ref_dir(): return early if directory cannot be read
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
@ 2012-04-26 22:26 ` mhagger
2012-04-26 22:26 ` [PATCH v2 02/18] get_ref_dir(): use a strbuf to hold refname mhagger
` (16 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 85 +++++++++++++++++++++++++++++++++-------------------------------
1 file changed, 44 insertions(+), 41 deletions(-)
diff --git a/refs.c b/refs.c
index 09322fe..d539241 100644
--- a/refs.c
+++ b/refs.c
@@ -754,6 +754,9 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
{
DIR *d;
const char *path;
+ struct dirent *de;
+ int baselen;
+ char *refname;
if (*refs->name)
path = git_path_submodule(refs->name, "%s", base);
@@ -761,55 +764,55 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
path = git_path("%s", base);
d = opendir(path);
- if (d) {
- struct dirent *de;
- int baselen = strlen(base);
- char *refname = xmalloc(baselen + 257);
+ if (!d)
+ return;
- memcpy(refname, base, baselen);
- if (baselen && base[baselen-1] != '/')
- refname[baselen++] = '/';
+ baselen = strlen(base);
+ refname = xmalloc(baselen + 257);
- while ((de = readdir(d)) != NULL) {
- unsigned char sha1[20];
- struct stat st;
- int flag;
- int namelen;
- const char *refdir;
+ memcpy(refname, base, baselen);
+ if (baselen && base[baselen-1] != '/')
+ refname[baselen++] = '/';
- if (de->d_name[0] == '.')
- continue;
- namelen = strlen(de->d_name);
- if (namelen > 255)
- continue;
- if (has_extension(de->d_name, ".lock"))
- continue;
- memcpy(refname + baselen, de->d_name, namelen+1);
- refdir = *refs->name
- ? git_path_submodule(refs->name, "%s", refname)
- : git_path("%s", refname);
- if (stat(refdir, &st) < 0)
- continue;
- if (S_ISDIR(st.st_mode)) {
- get_ref_dir(refs, refname, dir);
- continue;
- }
- if (*refs->name) {
- hashclr(sha1);
- flag = 0;
- if (resolve_gitlink_ref(refs->name, refname, sha1) < 0) {
- hashclr(sha1);
- flag |= REF_ISBROKEN;
- }
- } else if (read_ref_full(refname, sha1, 1, &flag)) {
+ while ((de = readdir(d)) != NULL) {
+ unsigned char sha1[20];
+ struct stat st;
+ int flag;
+ int namelen;
+ const char *refdir;
+
+ if (de->d_name[0] == '.')
+ continue;
+ namelen = strlen(de->d_name);
+ if (namelen > 255)
+ continue;
+ if (has_extension(de->d_name, ".lock"))
+ continue;
+ memcpy(refname + baselen, de->d_name, namelen+1);
+ refdir = *refs->name
+ ? git_path_submodule(refs->name, "%s", refname)
+ : git_path("%s", refname);
+ if (stat(refdir, &st) < 0)
+ continue;
+ if (S_ISDIR(st.st_mode)) {
+ get_ref_dir(refs, refname, dir);
+ continue;
+ }
+ if (*refs->name) {
+ hashclr(sha1);
+ flag = 0;
+ if (resolve_gitlink_ref(refs->name, refname, sha1) < 0) {
hashclr(sha1);
flag |= REF_ISBROKEN;
}
- add_ref(dir, create_ref_entry(refname, sha1, flag, 1));
+ } else if (read_ref_full(refname, sha1, 1, &flag)) {
+ hashclr(sha1);
+ flag |= REF_ISBROKEN;
}
- free(refname);
- closedir(d);
+ add_ref(dir, create_ref_entry(refname, sha1, flag, 1));
}
+ free(refname);
+ closedir(d);
}
static struct ref_dir *get_loose_refs(struct ref_cache *refs)
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 02/18] get_ref_dir(): use a strbuf to hold refname
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
2012-04-26 22:26 ` [PATCH v2 01/18] get_ref_dir(): return early if directory cannot be read mhagger
@ 2012-04-26 22:26 ` mhagger
2012-04-26 22:26 ` [PATCH v2 03/18] get_ref_dir(): rename "base" parameter to "dirname" mhagger
` (15 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
This simplifies the bookkeeping and allows an (artificial) restriction
on refname component length to be removed.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 54 ++++++++++++++++++++++++++----------------------------
1 file changed, 26 insertions(+), 28 deletions(-)
diff --git a/refs.c b/refs.c
index d539241..df98622 100644
--- a/refs.c
+++ b/refs.c
@@ -756,7 +756,7 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
const char *path;
struct dirent *de;
int baselen;
- char *refname;
+ struct strbuf refname;
if (*refs->name)
path = git_path_submodule(refs->name, "%s", base);
@@ -768,50 +768,48 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
return;
baselen = strlen(base);
- refname = xmalloc(baselen + 257);
-
- memcpy(refname, base, baselen);
- if (baselen && base[baselen-1] != '/')
- refname[baselen++] = '/';
+ strbuf_init(&refname, baselen + 257);
+ strbuf_add(&refname, base, baselen);
+ if (baselen && base[baselen-1] != '/') {
+ strbuf_addch(&refname, '/');
+ baselen++;
+ }
while ((de = readdir(d)) != NULL) {
unsigned char sha1[20];
struct stat st;
int flag;
- int namelen;
const char *refdir;
if (de->d_name[0] == '.')
continue;
- namelen = strlen(de->d_name);
- if (namelen > 255)
- continue;
if (has_extension(de->d_name, ".lock"))
continue;
- memcpy(refname + baselen, de->d_name, namelen+1);
+ strbuf_addstr(&refname, de->d_name);
refdir = *refs->name
- ? git_path_submodule(refs->name, "%s", refname)
- : git_path("%s", refname);
- if (stat(refdir, &st) < 0)
- continue;
- if (S_ISDIR(st.st_mode)) {
- get_ref_dir(refs, refname, dir);
- continue;
- }
- if (*refs->name) {
- hashclr(sha1);
- flag = 0;
- if (resolve_gitlink_ref(refs->name, refname, sha1) < 0) {
+ ? git_path_submodule(refs->name, "%s", refname.buf)
+ : git_path("%s", refname.buf);
+ if (stat(refdir, &st) < 0) {
+ /* Silently ignore. */
+ } else if (S_ISDIR(st.st_mode)) {
+ get_ref_dir(refs, refname.buf, dir);
+ } else {
+ if (*refs->name) {
+ hashclr(sha1);
+ flag = 0;
+ if (resolve_gitlink_ref(refs->name, refname.buf, sha1) < 0) {
+ hashclr(sha1);
+ flag |= REF_ISBROKEN;
+ }
+ } else if (read_ref_full(refname.buf, sha1, 1, &flag)) {
hashclr(sha1);
flag |= REF_ISBROKEN;
}
- } else if (read_ref_full(refname, sha1, 1, &flag)) {
- hashclr(sha1);
- flag |= REF_ISBROKEN;
+ add_ref(dir, create_ref_entry(refname.buf, sha1, flag, 1));
}
- add_ref(dir, create_ref_entry(refname, sha1, flag, 1));
+ strbuf_setlen(&refname, baselen);
}
- free(refname);
+ strbuf_release(&refname);
closedir(d);
}
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 03/18] get_ref_dir(): rename "base" parameter to "dirname"
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
2012-04-26 22:26 ` [PATCH v2 01/18] get_ref_dir(): return early if directory cannot be read mhagger
2012-04-26 22:26 ` [PATCH v2 02/18] get_ref_dir(): use a strbuf to hold refname mhagger
@ 2012-04-26 22:26 ` mhagger
2012-04-26 22:26 ` [PATCH v2 04/18] get_ref_dir(): require that the dirname argument ends in '/' mhagger
` (14 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index df98622..dc2b4df 100644
--- a/refs.c
+++ b/refs.c
@@ -749,30 +749,30 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)
create_ref_entry(refname, sha1, REF_ISPACKED, 1));
}
-static void get_ref_dir(struct ref_cache *refs, const char *base,
+static void get_ref_dir(struct ref_cache *refs, const char *dirname,
struct ref_dir *dir)
{
DIR *d;
const char *path;
struct dirent *de;
- int baselen;
+ int dirnamelen;
struct strbuf refname;
if (*refs->name)
- path = git_path_submodule(refs->name, "%s", base);
+ path = git_path_submodule(refs->name, "%s", dirname);
else
- path = git_path("%s", base);
+ path = git_path("%s", dirname);
d = opendir(path);
if (!d)
return;
- baselen = strlen(base);
- strbuf_init(&refname, baselen + 257);
- strbuf_add(&refname, base, baselen);
- if (baselen && base[baselen-1] != '/') {
+ dirnamelen = strlen(dirname);
+ strbuf_init(&refname, dirnamelen + 257);
+ strbuf_add(&refname, dirname, dirnamelen);
+ if (dirnamelen && dirname[dirnamelen-1] != '/') {
strbuf_addch(&refname, '/');
- baselen++;
+ dirnamelen++;
}
while ((de = readdir(d)) != NULL) {
@@ -807,7 +807,7 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
}
add_ref(dir, create_ref_entry(refname.buf, sha1, flag, 1));
}
- strbuf_setlen(&refname, baselen);
+ strbuf_setlen(&refname, dirnamelen);
}
strbuf_release(&refname);
closedir(d);
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 04/18] get_ref_dir(): require that the dirname argument ends in '/'
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (2 preceding siblings ...)
2012-04-26 22:26 ` [PATCH v2 03/18] get_ref_dir(): rename "base" parameter to "dirname" mhagger
@ 2012-04-26 22:26 ` mhagger
2012-04-26 22:26 ` [PATCH v2 05/18] refs.c: extract function search_for_subdir() mhagger
` (13 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
This removes some conditional code and makes it consistent with the
way that direntry names are stored. Please note that this function is
never used on the top-level .git directory; it is always called for
directories at level .git/refs or deeper.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index dc2b4df..01fcdc7 100644
--- a/refs.c
+++ b/refs.c
@@ -749,13 +749,17 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)
create_ref_entry(refname, sha1, REF_ISPACKED, 1));
}
+/*
+ * Read the loose references for refs from the namespace dirname.
+ * dirname must end with '/'.
+ */
static void get_ref_dir(struct ref_cache *refs, const char *dirname,
struct ref_dir *dir)
{
DIR *d;
const char *path;
struct dirent *de;
- int dirnamelen;
+ int dirnamelen = strlen(dirname);
struct strbuf refname;
if (*refs->name)
@@ -767,13 +771,8 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,
if (!d)
return;
- dirnamelen = strlen(dirname);
strbuf_init(&refname, dirnamelen + 257);
strbuf_add(&refname, dirname, dirnamelen);
- if (dirnamelen && dirname[dirnamelen-1] != '/') {
- strbuf_addch(&refname, '/');
- dirnamelen++;
- }
while ((de = readdir(d)) != NULL) {
unsigned char sha1[20];
@@ -792,6 +791,7 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,
if (stat(refdir, &st) < 0) {
/* Silently ignore. */
} else if (S_ISDIR(st.st_mode)) {
+ strbuf_addch(&refname, '/');
get_ref_dir(refs, refname.buf, dir);
} else {
if (*refs->name) {
@@ -816,7 +816,7 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,
static struct ref_dir *get_loose_refs(struct ref_cache *refs)
{
if (!refs->did_loose) {
- get_ref_dir(refs, "refs", &refs->loose);
+ get_ref_dir(refs, "refs/", &refs->loose);
refs->did_loose = 1;
}
return &refs->loose;
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 05/18] refs.c: extract function search_for_subdir()
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (3 preceding siblings ...)
2012-04-26 22:26 ` [PATCH v2 04/18] get_ref_dir(): require that the dirname argument ends in '/' mhagger
@ 2012-04-26 22:26 ` mhagger
2012-05-03 19:48 ` Junio C Hamano
2012-04-26 22:26 ` [PATCH v2 06/18] get_ref_dir(): take the containing directory as argument mhagger
` (12 subsequent siblings)
17 siblings, 1 reply; 26+ messages in thread
From: mhagger @ 2012-04-26 22:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index 01fcdc7..5e51c10 100644
--- a/refs.c
+++ b/refs.c
@@ -277,6 +277,27 @@ static struct ref_entry *search_ref_dir(struct ref_dir *dir, const char *refname
}
/*
+ * Search for a directory entry directly within dir (without
+ * recursing). Sort dir if necessary. subdirname must be a directory
+ * name (i.e., end in '/'). If mkdir is set, then create the
+ * directory if it is missing; otherwise, return NULL if the desired
+ * directory cannot be found.
+ */
+static struct ref_entry *search_for_subdir(struct ref_dir *dir,
+ const char *subdirname, int mkdir)
+{
+ struct ref_entry *entry = search_ref_dir(dir, subdirname);
+ if (!entry) {
+ if (!mkdir)
+ return NULL;
+ entry = create_dir_entry(subdirname);
+ add_entry_to_dir(dir, entry);
+ }
+ assert(entry->flag & REF_DIR);
+ return entry;
+}
+
+/*
* If refname is a reference name, find the ref_dir within the dir
* tree that should hold refname. If refname is a directory name
* (i.e., ends in '/'), then return that ref_dir itself. dir must
@@ -294,17 +315,10 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,
for (slash = strchr(refname_copy, '/'); slash; slash = strchr(slash + 1, '/')) {
char tmp = slash[1];
slash[1] = '\0';
- entry = search_ref_dir(dir, refname_copy);
- if (!entry) {
- if (!mkdir) {
- dir = NULL;
- break;
- }
- entry = create_dir_entry(refname_copy);
- add_entry_to_dir(dir, entry);
- }
+ entry = search_for_subdir(dir, refname_copy, mkdir);
slash[1] = tmp;
- assert(entry->flag & REF_DIR);
+ if (!entry)
+ break;
dir = &entry->u.subdir;
}
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v2 05/18] refs.c: extract function search_for_subdir()
2012-04-26 22:26 ` [PATCH v2 05/18] refs.c: extract function search_for_subdir() mhagger
@ 2012-05-03 19:48 ` Junio C Hamano
2012-05-03 20:56 ` Junio C Hamano
0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2012-05-03 19:48 UTC (permalink / raw)
To: mhagger
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder
mhagger@alum.mit.edu writes:
> From: Michael Haggerty <mhagger@alum.mit.edu>
>
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> refs.c | 34 ++++++++++++++++++++++++----------
> 1 file changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 01fcdc7..5e51c10 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -277,6 +277,27 @@ static struct ref_entry *search_ref_dir(struct ref_dir *dir, const char *refname
> }
>
> /*
> + * Search for a directory entry directly within dir (without
> + * recursing). Sort dir if necessary. subdirname must be a directory
> + * name (i.e., end in '/'). If mkdir is set, then create the
> + * directory if it is missing; otherwise, return NULL if the desired
> + * directory cannot be found.
> + */
> +static struct ref_entry *search_for_subdir(struct ref_dir *dir,
> + const char *subdirname, int mkdir)
> +{
> + struct ref_entry *entry = search_ref_dir(dir, subdirname);
> + if (!entry) {
> + if (!mkdir)
> + return NULL;
> + entry = create_dir_entry(subdirname);
> + add_entry_to_dir(dir, entry);
> + }
> + assert(entry->flag & REF_DIR);
> + return entry;
> +}
> +
> +/*
> * If refname is a reference name, find the ref_dir within the dir
> * tree that should hold refname. If refname is a directory name
> * (i.e., ends in '/'), then return that ref_dir itself. dir must
> @@ -294,17 +315,10 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,
> for (slash = strchr(refname_copy, '/'); slash; slash = strchr(slash + 1, '/')) {
> char tmp = slash[1];
> slash[1] = '\0';
> - entry = search_ref_dir(dir, refname_copy);
> - if (!entry) {
> - if (!mkdir) {
> - dir = NULL;
> - break;
> - }
> - entry = create_dir_entry(refname_copy);
> - add_entry_to_dir(dir, entry);
> - }
> + entry = search_for_subdir(dir, refname_copy, mkdir);
> slash[1] = tmp;
> - assert(entry->flag & REF_DIR);
> + if (!entry)
> + break;
> dir = &entry->u.subdir;
> }
Hrm. The old code used to reset "dir" to NULL before breaking, so the
entire function used to return NULL. Now, it calls search_for_subdir(),
which calls search_ref_dir() and gets NULL in entry, and returns NULL.
Wouldn't we end up returning the original parameter "dir" instead of NULL
in that case? Would that make a difference?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 05/18] refs.c: extract function search_for_subdir()
2012-05-03 19:48 ` Junio C Hamano
@ 2012-05-03 20:56 ` Junio C Hamano
2012-05-04 7:24 ` Michael Haggerty
0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2012-05-03 20:56 UTC (permalink / raw)
To: mhagger
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder
Junio C Hamano <gitster@pobox.com> writes:
> Hrm. The old code used to reset "dir" to NULL before breaking, so the
> entire function used to return NULL. Now, it calls search_for_subdir(),
> which calls search_ref_dir() and gets NULL in entry, and returns NULL.
>
> Wouldn't we end up returning the original parameter "dir" instead of NULL
> in that case? Would that make a difference?
In other words, isn't something like this necessary?
Otherwise, wouldn't do_for_each_ref() called for a non-existing "refs/"
subhierarchy in "base" start from the top-level packed_dir/loose_dir
returned from find_containing_dir(), and end up running do_for_each_ref_in_dirs()
with both top-level packed_dir/loose_dir and traversing all of them, only
to find nothing?
refs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 9f2da16..af5da5f 100644
--- a/refs.c
+++ b/refs.c
@@ -390,8 +390,10 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,
refname + dirname.len,
(slash + 1) - (refname + dirname.len));
subdir = search_for_subdir(dir, dirname.buf, mkdir);
- if (!subdir)
+ if (!subdir) {
+ dir = NULL;
break;
+ }
dir = subdir;
}
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v2 05/18] refs.c: extract function search_for_subdir()
2012-05-03 20:56 ` Junio C Hamano
@ 2012-05-04 7:24 ` Michael Haggerty
0 siblings, 0 replies; 26+ messages in thread
From: Michael Haggerty @ 2012-05-04 7:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder
On 05/03/2012 10:56 PM, Junio C Hamano wrote:
> Junio C Hamano<gitster@pobox.com> writes:
>
>> Hrm. The old code used to reset "dir" to NULL before breaking, so the
>> entire function used to return NULL. Now, it calls search_for_subdir(),
>> which calls search_ref_dir() and gets NULL in entry, and returns NULL.
>>
>> Wouldn't we end up returning the original parameter "dir" instead of NULL
>> in that case? Would that make a difference?
>
> In other words, isn't something like this necessary?
You are right. Thanks for catching this.
> Otherwise, wouldn't do_for_each_ref() called for a non-existing "refs/"
> subhierarchy in "base" start from the top-level packed_dir/loose_dir
> returned from find_containing_dir(), and end up running do_for_each_ref_in_dirs()
> with both top-level packed_dir/loose_dir and traversing all of them, only
> to find nothing?
>
> refs.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/refs.c b/refs.c
> index 9f2da16..af5da5f 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -390,8 +390,10 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,
> refname + dirname.len,
> (slash + 1) - (refname + dirname.len));
> subdir = search_for_subdir(dir, dirname.buf, mkdir);
> - if (!subdir)
> + if (!subdir) {
> + dir = NULL;
> break;
> + }
> dir = subdir;
> }
>
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 06/18] get_ref_dir(): take the containing directory as argument
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (4 preceding siblings ...)
2012-04-26 22:26 ` [PATCH v2 05/18] refs.c: extract function search_for_subdir() mhagger
@ 2012-04-26 22:26 ` mhagger
2012-04-26 22:26 ` [PATCH v2 07/18] do_for_each_reflog(): return early on error mhagger
` (11 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Previously, the "dir" argument to get_ref_dir() was a pointer to the
top-level ref_dir. Change the function to expect a pointer to the
ref_dir corresponding to dirname. This allows entries to be added
directly to dir, without having to recurse through the reference trie
each time (i.e., we can use add_entry_to_dir() instead of add_ref()).
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 5e51c10..8670c2e 100644
--- a/refs.c
+++ b/refs.c
@@ -765,7 +765,8 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)
/*
* Read the loose references for refs from the namespace dirname.
- * dirname must end with '/'.
+ * dirname must end with '/'. dir must be the directory entry
+ * corresponding to dirname.
*/
static void get_ref_dir(struct ref_cache *refs, const char *dirname,
struct ref_dir *dir)
@@ -806,7 +807,8 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,
/* Silently ignore. */
} else if (S_ISDIR(st.st_mode)) {
strbuf_addch(&refname, '/');
- get_ref_dir(refs, refname.buf, dir);
+ get_ref_dir(refs, refname.buf,
+ &search_for_subdir(dir, refname.buf, 1)->u.subdir);
} else {
if (*refs->name) {
hashclr(sha1);
@@ -819,7 +821,8 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,
hashclr(sha1);
flag |= REF_ISBROKEN;
}
- add_ref(dir, create_ref_entry(refname.buf, sha1, flag, 1));
+ add_entry_to_dir(dir,
+ create_ref_entry(refname.buf, sha1, flag, 1));
}
strbuf_setlen(&refname, dirnamelen);
}
@@ -830,7 +833,8 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,
static struct ref_dir *get_loose_refs(struct ref_cache *refs)
{
if (!refs->did_loose) {
- get_ref_dir(refs, "refs/", &refs->loose);
+ get_ref_dir(refs, "refs/",
+ &search_for_subdir(&refs->loose, "refs/", 1)->u.subdir);
refs->did_loose = 1;
}
return &refs->loose;
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 07/18] do_for_each_reflog(): return early on error
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (5 preceding siblings ...)
2012-04-26 22:26 ` [PATCH v2 06/18] get_ref_dir(): take the containing directory as argument mhagger
@ 2012-04-26 22:26 ` mhagger
2012-04-26 22:26 ` [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name mhagger
` (10 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 70 ++++++++++++++++++++++++++++++++--------------------------------
1 file changed, 35 insertions(+), 35 deletions(-)
diff --git a/refs.c b/refs.c
index 8670c2e..1d25151 100644
--- a/refs.c
+++ b/refs.c
@@ -2246,47 +2246,47 @@ static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
{
DIR *d = opendir(git_path("logs/%s", base));
int retval = 0;
+ struct dirent *de;
+ int baselen;
+ char *log;
- if (d) {
- struct dirent *de;
- int baselen = strlen(base);
- char *log = xmalloc(baselen + 257);
+ if (!d)
+ return *base ? errno : 0;
- memcpy(log, base, baselen);
- if (baselen && base[baselen-1] != '/')
- log[baselen++] = '/';
+ baselen = strlen(base);
+ log = xmalloc(baselen + 257);
+ memcpy(log, base, baselen);
+ if (baselen && base[baselen-1] != '/')
+ log[baselen++] = '/';
- while ((de = readdir(d)) != NULL) {
- struct stat st;
- int namelen;
+ while ((de = readdir(d)) != NULL) {
+ struct stat st;
+ int namelen;
- if (de->d_name[0] == '.')
- continue;
- namelen = strlen(de->d_name);
- if (namelen > 255)
- continue;
- if (has_extension(de->d_name, ".lock"))
- continue;
- memcpy(log + baselen, de->d_name, namelen+1);
- if (stat(git_path("logs/%s", log), &st) < 0)
- continue;
- if (S_ISDIR(st.st_mode)) {
- retval = do_for_each_reflog(log, fn, cb_data);
- } else {
- unsigned char sha1[20];
- if (read_ref_full(log, sha1, 0, NULL))
- retval = error("bad ref for %s", log);
- else
- retval = fn(log, sha1, 0, cb_data);
- }
- if (retval)
- break;
+ if (de->d_name[0] == '.')
+ continue;
+ namelen = strlen(de->d_name);
+ if (namelen > 255)
+ continue;
+ if (has_extension(de->d_name, ".lock"))
+ continue;
+ memcpy(log + baselen, de->d_name, namelen+1);
+ if (stat(git_path("logs/%s", log), &st) < 0)
+ continue;
+ if (S_ISDIR(st.st_mode)) {
+ retval = do_for_each_reflog(log, fn, cb_data);
+ } else {
+ unsigned char sha1[20];
+ if (read_ref_full(log, sha1, 0, NULL))
+ retval = error("bad ref for %s", log);
+ else
+ retval = fn(log, sha1, 0, cb_data);
}
- free(log);
- closedir(d);
+ if (retval)
+ break;
}
- else if (*base)
- return errno;
+ free(log);
+ closedir(d);
return retval;
}
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (6 preceding siblings ...)
2012-04-26 22:26 ` [PATCH v2 07/18] do_for_each_reflog(): return early on error mhagger
@ 2012-04-26 22:26 ` mhagger
2012-04-26 23:25 ` Junio C Hamano
2012-04-26 22:26 ` [PATCH v2 09/18] do_for_each_reflog(): reuse strbuf across recursive function calls mhagger
` (9 subsequent siblings)
17 siblings, 1 reply; 26+ messages in thread
From: mhagger @ 2012-04-26 22:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
This simplifies the bookkeeping and allows an (artificial) restriction
on refname component length to be removed.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 45 +++++++++++++++++++++++----------------------
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/refs.c b/refs.c
index 1d25151..f43c255 100644
--- a/refs.c
+++ b/refs.c
@@ -2248,44 +2248,45 @@ static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
int retval = 0;
struct dirent *de;
int baselen;
- char *log;
+ struct strbuf log;
if (!d)
return *base ? errno : 0;
baselen = strlen(base);
- log = xmalloc(baselen + 257);
- memcpy(log, base, baselen);
- if (baselen && base[baselen-1] != '/')
- log[baselen++] = '/';
+ strbuf_init(&log, baselen + 257);
+ strbuf_add(&log, base, baselen);
+ if (log.len && log.buf[log.len-1] != '/') {
+ strbuf_addch(&log, '/');
+ baselen++;
+ }
while ((de = readdir(d)) != NULL) {
struct stat st;
- int namelen;
if (de->d_name[0] == '.')
continue;
- namelen = strlen(de->d_name);
- if (namelen > 255)
- continue;
if (has_extension(de->d_name, ".lock"))
continue;
- memcpy(log + baselen, de->d_name, namelen+1);
- if (stat(git_path("logs/%s", log), &st) < 0)
- continue;
- if (S_ISDIR(st.st_mode)) {
- retval = do_for_each_reflog(log, fn, cb_data);
+ strbuf_addstr(&log, de->d_name);
+ if (stat(git_path("logs/%s", log.buf), &st) < 0) {
+ /* Silently ignore. */
} else {
- unsigned char sha1[20];
- if (read_ref_full(log, sha1, 0, NULL))
- retval = error("bad ref for %s", log);
- else
- retval = fn(log, sha1, 0, cb_data);
+ if (S_ISDIR(st.st_mode)) {
+ retval = do_for_each_reflog(log.buf, fn, cb_data);
+ } else {
+ unsigned char sha1[20];
+ if (read_ref_full(log.buf, sha1, 0, NULL))
+ retval = error("bad ref for %s", log.buf);
+ else
+ retval = fn(log.buf, sha1, 0, cb_data);
+ }
+ if (retval)
+ break;
}
- if (retval)
- break;
+ strbuf_setlen(&log, baselen);
}
- free(log);
+ strbuf_release(&log);
closedir(d);
return retval;
}
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name
2012-04-26 22:26 ` [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name mhagger
@ 2012-04-26 23:25 ` Junio C Hamano
2012-04-27 8:59 ` Michael Haggerty
0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2012-04-26 23:25 UTC (permalink / raw)
To: mhagger
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder
mhagger@alum.mit.edu writes:
> From: Michael Haggerty <mhagger@alum.mit.edu>
>
> This simplifies the bookkeeping and allows an (artificial) restriction
> on refname component length to be removed.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> refs.c | 45 +++++++++++++++++++++++----------------------
> 1 file changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 1d25151..f43c255 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2248,44 +2248,45 @@ static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
> int retval = 0;
> struct dirent *de;
> int baselen;
> - char *log;
> + struct strbuf log;
>
> if (!d)
> return *base ? errno : 0;
>
> baselen = strlen(base);
> - log = xmalloc(baselen + 257);
> - memcpy(log, base, baselen);
> - if (baselen && base[baselen-1] != '/')
> - log[baselen++] = '/';
> + strbuf_init(&log, baselen + 257);
> + strbuf_add(&log, base, baselen);
> + if (log.len && log.buf[log.len-1] != '/') {
> + strbuf_addch(&log, '/');
> + baselen++;
> + }
>
> while ((de = readdir(d)) != NULL) {
> struct stat st;
> - int namelen;
>
> if (de->d_name[0] == '.')
> continue;
> - namelen = strlen(de->d_name);
> - if (namelen > 255)
> - continue;
> if (has_extension(de->d_name, ".lock"))
> continue;
> - memcpy(log + baselen, de->d_name, namelen+1);
> - if (stat(git_path("logs/%s", log), &st) < 0)
> - continue;
> - if (S_ISDIR(st.st_mode)) {
> - retval = do_for_each_reflog(log, fn, cb_data);
> + strbuf_addstr(&log, de->d_name);
> + if (stat(git_path("logs/%s", log.buf), &st) < 0) {
> + /* Silently ignore. */
> } else {
Please write this like this:
if (...) {
; /* silently ignore */
}
to make the "emptyness" stand out (I amended the previous round when I
queued them to 'pu', but I forgot to point it out in my review message).
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name
2012-04-26 23:25 ` Junio C Hamano
@ 2012-04-27 8:59 ` Michael Haggerty
2012-05-02 20:06 ` Junio C Hamano
0 siblings, 1 reply; 26+ messages in thread
From: Michael Haggerty @ 2012-04-27 8:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder
On 04/27/2012 01:25 AM, Junio C Hamano wrote:
> Please write this like this:
>
> if (...) {
> ; /* silently ignore */
> }
>
> to make the "emptyness" stand out (I amended the previous round when I
> queued them to 'pu', but I forgot to point it out in my review message).
OK. A similar construct is in patch 2 of the same series. I've fixed
them in my repo and will use the fixed versions if there are any future
re-rolls.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name
2012-04-27 8:59 ` Michael Haggerty
@ 2012-05-02 20:06 ` Junio C Hamano
2012-05-03 6:47 ` Michael Haggerty
0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2012-05-02 20:06 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 04/27/2012 01:25 AM, Junio C Hamano wrote:
>> Please write this like this:
>>
>> if (...) {
>> ; /* silently ignore */
>> }
>>
>> to make the "emptyness" stand out (I amended the previous round when I
>> queued them to 'pu', but I forgot to point it out in my review message).
>
> OK. A similar construct is in patch 2 of the same series. I've fixed
> them in my repo and will use the fixed versions if there are any
> future re-rolls.
OK. Has there been any re-roll I missed, or is what I have in 'pu' with
fixups ready for 'next'?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name
2012-05-02 20:06 ` Junio C Hamano
@ 2012-05-03 6:47 ` Michael Haggerty
0 siblings, 0 replies; 26+ messages in thread
From: Michael Haggerty @ 2012-05-03 6:47 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder
On 05/02/2012 10:06 PM, Junio C Hamano wrote:
> Michael Haggerty<mhagger@alum.mit.edu> writes:
>
>> On 04/27/2012 01:25 AM, Junio C Hamano wrote:
>>> Please write this like this:
>>>
>>> if (...) {
>>> ; /* silently ignore */
>>> }
>>>
>>> to make the "emptyness" stand out (I amended the previous round when I
>>> queued them to 'pu', but I forgot to point it out in my review message).
>>
>> OK. A similar construct is in patch 2 of the same series. I've fixed
>> them in my repo and will use the fixed versions if there are any
>> future re-rolls.
>
> OK. Has there been any re-roll I missed, or is what I have in 'pu' with
> fixups ready for 'next'?
I haven't gotten any other feedback, so as far as I'm concerned, it's
ready for next.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 09/18] do_for_each_reflog(): reuse strbuf across recursive function calls
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (7 preceding siblings ...)
2012-04-26 22:26 ` [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name mhagger
@ 2012-04-26 22:26 ` mhagger
2012-04-26 22:26 ` [PATCH v2 10/18] bisect: copy filename string obtained from git_path() mhagger
` (8 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
This saves some memory allocations. Also require that the name
argument end in slash, which removes some extra bookkeeping.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 45 +++++++++++++++++++++++----------------------
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/refs.c b/refs.c
index f43c255..989c10d 100644
--- a/refs.c
+++ b/refs.c
@@ -2242,24 +2242,20 @@ int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_dat
return for_each_recent_reflog_ent(refname, fn, 0, cb_data);
}
-static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
+/*
+ * Call fn for each reflog in the namespace indicated by name. name
+ * must be empty or end with '/'. Name will be used as a scratch
+ * space, but its contents will be restored before return.
+ */
+static int do_for_each_reflog(struct strbuf *name, each_ref_fn fn, void *cb_data)
{
- DIR *d = opendir(git_path("logs/%s", base));
+ DIR *d = opendir(git_path("logs/%s", name->buf));
int retval = 0;
struct dirent *de;
- int baselen;
- struct strbuf log;
+ int oldlen = name->len;
if (!d)
- return *base ? errno : 0;
-
- baselen = strlen(base);
- strbuf_init(&log, baselen + 257);
- strbuf_add(&log, base, baselen);
- if (log.len && log.buf[log.len-1] != '/') {
- strbuf_addch(&log, '/');
- baselen++;
- }
+ return name->len ? errno : 0;
while ((de = readdir(d)) != NULL) {
struct stat st;
@@ -2268,32 +2264,37 @@ static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
continue;
if (has_extension(de->d_name, ".lock"))
continue;
- strbuf_addstr(&log, de->d_name);
- if (stat(git_path("logs/%s", log.buf), &st) < 0) {
+ strbuf_addstr(name, de->d_name);
+ if (stat(git_path("logs/%s", name->buf), &st) < 0) {
/* Silently ignore. */
} else {
if (S_ISDIR(st.st_mode)) {
- retval = do_for_each_reflog(log.buf, fn, cb_data);
+ strbuf_addch(name, '/');
+ retval = do_for_each_reflog(name, fn, cb_data);
} else {
unsigned char sha1[20];
- if (read_ref_full(log.buf, sha1, 0, NULL))
- retval = error("bad ref for %s", log.buf);
+ if (read_ref_full(name->buf, sha1, 0, NULL))
+ retval = error("bad ref for %s", name->buf);
else
- retval = fn(log.buf, sha1, 0, cb_data);
+ retval = fn(name->buf, sha1, 0, cb_data);
}
if (retval)
break;
}
- strbuf_setlen(&log, baselen);
+ strbuf_setlen(name, oldlen);
}
- strbuf_release(&log);
closedir(d);
return retval;
}
int for_each_reflog(each_ref_fn fn, void *cb_data)
{
- return do_for_each_reflog("", fn, cb_data);
+ int retval;
+ struct strbuf name;
+ strbuf_init(&name, PATH_MAX);
+ retval = do_for_each_reflog(&name, fn, cb_data);
+ strbuf_release(&name);
+ return retval;
}
int update_ref(const char *action, const char *refname,
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 10/18] bisect: copy filename string obtained from git_path()
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (8 preceding siblings ...)
2012-04-26 22:26 ` [PATCH v2 09/18] do_for_each_reflog(): reuse strbuf across recursive function calls mhagger
@ 2012-04-26 22:26 ` mhagger
2012-04-26 22:27 ` [PATCH v2 11/18] find_containing_dir(): use strbuf in implementation of this function mhagger
` (7 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Prevent the string from being overwritten by other callers of
git_path() and friends before we are done using it.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Otherwise, this bug will be triggered by later patches in the series.
I didn't do a careful code audit of this problem, but it seems very
plausible that that check_ancestors() and/or check_merge_bases() are
guilty. They certainly do a lot more than should be done while
holding on to a pointer to a statically-allocated buffer.
I cursorily checked other code in the neighborhood for similar abuses,
but it would be good for an expert to look it over.
bisect.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/bisect.c b/bisect.c
index 6e186e2..48acf73 100644
--- a/bisect.c
+++ b/bisect.c
@@ -833,7 +833,7 @@ static int check_ancestors(const char *prefix)
*/
static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)
{
- const char *filename = git_path("BISECT_ANCESTORS_OK");
+ char *filename = xstrdup(git_path("BISECT_ANCESTORS_OK"));
struct stat st;
int fd;
@@ -842,11 +842,11 @@ static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)
/* Check if file BISECT_ANCESTORS_OK exists. */
if (!stat(filename, &st) && S_ISREG(st.st_mode))
- return;
+ goto done;
/* Bisecting with no good rev is ok. */
if (good_revs.nr == 0)
- return;
+ goto done;
/* Check if all good revs are ancestor of the bad rev. */
if (check_ancestors(prefix))
@@ -859,6 +859,8 @@ static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)
filename, strerror(errno));
else
close(fd);
+ done:
+ free(filename);
}
/*
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 11/18] find_containing_dir(): use strbuf in implementation of this function
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (9 preceding siblings ...)
2012-04-26 22:26 ` [PATCH v2 10/18] bisect: copy filename string obtained from git_path() mhagger
@ 2012-04-26 22:27 ` mhagger
2012-04-26 22:27 ` [PATCH v2 12/18] refs: wrap top-level ref_dirs in ref_entries mhagger
` (6 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 989c10d..94b20a3 100644
--- a/refs.c
+++ b/refs.c
@@ -309,20 +309,21 @@ static struct ref_entry *search_for_subdir(struct ref_dir *dir,
static struct ref_dir *find_containing_dir(struct ref_dir *dir,
const char *refname, int mkdir)
{
- char *refname_copy = xstrdup(refname);
- char *slash;
- struct ref_entry *entry;
- for (slash = strchr(refname_copy, '/'); slash; slash = strchr(slash + 1, '/')) {
- char tmp = slash[1];
- slash[1] = '\0';
- entry = search_for_subdir(dir, refname_copy, mkdir);
- slash[1] = tmp;
+ struct strbuf dirname;
+ const char *slash;
+ strbuf_init(&dirname, PATH_MAX);
+ for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {
+ struct ref_entry *entry;
+ strbuf_add(&dirname,
+ refname + dirname.len,
+ (slash + 1) - (refname + dirname.len));
+ entry = search_for_subdir(dir, dirname.buf, mkdir);
if (!entry)
break;
dir = &entry->u.subdir;
}
- free(refname_copy);
+ strbuf_release(&dirname);
return dir;
}
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 12/18] refs: wrap top-level ref_dirs in ref_entries
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (10 preceding siblings ...)
2012-04-26 22:27 ` [PATCH v2 11/18] find_containing_dir(): use strbuf in implementation of this function mhagger
@ 2012-04-26 22:27 ` mhagger
2012-04-26 22:27 ` [PATCH v2 13/18] read_loose_refs(): rename function from get_ref_dir() mhagger
` (5 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Make it turtles all the way down. This affects the loose and packed
fields of ref_cache instances.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
This change is not crucial, as it was in v1 of the patch series.
Nevertheless I think that it improves the code consistency.
refs.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/refs.c b/refs.c
index 94b20a3..de14e75 100644
--- a/refs.c
+++ b/refs.c
@@ -607,26 +607,26 @@ static int is_refname_available(const char *refname, const char *oldrefname,
*/
static struct ref_cache {
struct ref_cache *next;
- char did_loose;
- char did_packed;
- struct ref_dir loose;
- struct ref_dir packed;
+ struct ref_entry *loose;
+ struct ref_entry *packed;
/* The submodule name, or "" for the main repo. */
char name[FLEX_ARRAY];
} *ref_cache;
static void clear_packed_ref_cache(struct ref_cache *refs)
{
- if (refs->did_packed)
- clear_ref_dir(&refs->packed);
- refs->did_packed = 0;
+ if (refs->packed) {
+ free_ref_entry(refs->packed);
+ refs->packed = NULL;
+ }
}
static void clear_loose_ref_cache(struct ref_cache *refs)
{
- if (refs->did_loose)
- clear_ref_dir(&refs->loose);
- refs->did_loose = 0;
+ if (refs->loose) {
+ free_ref_entry(refs->loose);
+ refs->loose = NULL;
+ }
}
static struct ref_cache *create_ref_cache(const char *submodule)
@@ -740,22 +740,22 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
static struct ref_dir *get_packed_refs(struct ref_cache *refs)
{
- if (!refs->did_packed) {
+ if (!refs->packed) {
const char *packed_refs_file;
FILE *f;
+ refs->packed = create_dir_entry("");
if (*refs->name)
packed_refs_file = git_path_submodule(refs->name, "packed-refs");
else
packed_refs_file = git_path("packed-refs");
f = fopen(packed_refs_file, "r");
if (f) {
- read_packed_refs(f, &refs->packed);
+ read_packed_refs(f, &refs->packed->u.subdir);
fclose(f);
}
- refs->did_packed = 1;
}
- return &refs->packed;
+ return &refs->packed->u.subdir;
}
void add_packed_ref(const char *refname, const unsigned char *sha1)
@@ -833,12 +833,13 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,
static struct ref_dir *get_loose_refs(struct ref_cache *refs)
{
- if (!refs->did_loose) {
+ if (!refs->loose) {
+ refs->loose = create_dir_entry("");
get_ref_dir(refs, "refs/",
- &search_for_subdir(&refs->loose, "refs/", 1)->u.subdir);
- refs->did_loose = 1;
+ &search_for_subdir(&refs->loose->u.subdir,
+ "refs/", 1)->u.subdir);
}
- return &refs->loose;
+ return &refs->loose->u.subdir;
}
/* We allow "recursive" symbolic refs. Only within reason, though */
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 13/18] read_loose_refs(): rename function from get_ref_dir()
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (11 preceding siblings ...)
2012-04-26 22:27 ` [PATCH v2 12/18] refs: wrap top-level ref_dirs in ref_entries mhagger
@ 2012-04-26 22:27 ` mhagger
2012-04-26 22:27 ` [PATCH v2 14/18] get_ref_dir(): add function for getting a ref_dir from a ref_entry mhagger
` (4 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
The new name better describes the function's purpose, and also makes
the old name available for a more suitable purpose.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index de14e75..1d18ae6 100644
--- a/refs.c
+++ b/refs.c
@@ -769,8 +769,8 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)
* dirname must end with '/'. dir must be the directory entry
* corresponding to dirname.
*/
-static void get_ref_dir(struct ref_cache *refs, const char *dirname,
- struct ref_dir *dir)
+static void read_loose_refs(struct ref_cache *refs, const char *dirname,
+ struct ref_dir *dir)
{
DIR *d;
const char *path;
@@ -808,8 +808,8 @@ static void get_ref_dir(struct ref_cache *refs, const char *dirname,
/* Silently ignore. */
} else if (S_ISDIR(st.st_mode)) {
strbuf_addch(&refname, '/');
- get_ref_dir(refs, refname.buf,
- &search_for_subdir(dir, refname.buf, 1)->u.subdir);
+ read_loose_refs(refs, refname.buf,
+ &search_for_subdir(dir, refname.buf, 1)->u.subdir);
} else {
if (*refs->name) {
hashclr(sha1);
@@ -835,9 +835,9 @@ static struct ref_dir *get_loose_refs(struct ref_cache *refs)
{
if (!refs->loose) {
refs->loose = create_dir_entry("");
- get_ref_dir(refs, "refs/",
- &search_for_subdir(&refs->loose->u.subdir,
- "refs/", 1)->u.subdir);
+ read_loose_refs(refs, "refs/",
+ &search_for_subdir(&refs->loose->u.subdir,
+ "refs/", 1)->u.subdir);
}
return &refs->loose->u.subdir;
}
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 14/18] get_ref_dir(): add function for getting a ref_dir from a ref_entry
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (12 preceding siblings ...)
2012-04-26 22:27 ` [PATCH v2 13/18] read_loose_refs(): rename function from get_ref_dir() mhagger
@ 2012-04-26 22:27 ` mhagger
2012-04-26 22:27 ` [PATCH v2 15/18] search_for_subdir(): return (ref_dir *) instead of (ref_entry *) mhagger
` (3 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Convert all accesses of a ref_dir within a ref_entry to use this
function. This function will later be responsible for reading loose
references from disk on demand.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 40 +++++++++++++++++++++++++---------------
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/refs.c b/refs.c
index 1d18ae6..2e71bfa 100644
--- a/refs.c
+++ b/refs.c
@@ -171,6 +171,12 @@ struct ref_entry {
char name[FLEX_ARRAY];
};
+static struct ref_dir *get_ref_dir(struct ref_entry *entry)
+{
+ assert(entry->flag & REF_DIR);
+ return &entry->u.subdir;
+}
+
static struct ref_entry *create_ref_entry(const char *refname,
const unsigned char *sha1, int flag,
int check_name)
@@ -195,7 +201,7 @@ static void clear_ref_dir(struct ref_dir *dir);
static void free_ref_entry(struct ref_entry *entry)
{
if (entry->flag & REF_DIR)
- clear_ref_dir(&entry->u.subdir);
+ clear_ref_dir(get_ref_dir(entry));
free(entry);
}
@@ -320,7 +326,7 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,
entry = search_for_subdir(dir, dirname.buf, mkdir);
if (!entry)
break;
- dir = &entry->u.subdir;
+ dir = get_ref_dir(entry);
}
strbuf_release(&dirname);
@@ -449,8 +455,9 @@ static int do_for_each_ref_in_dir(struct ref_dir *dir, int offset,
struct ref_entry *entry = dir->entries[i];
int retval;
if (entry->flag & REF_DIR) {
- sort_ref_dir(&entry->u.subdir);
- retval = do_for_each_ref_in_dir(&entry->u.subdir, 0,
+ struct ref_dir *subdir = get_ref_dir(entry);
+ sort_ref_dir(subdir);
+ retval = do_for_each_ref_in_dir(subdir, 0,
base, fn, trim, flags, cb_data);
} else {
retval = do_one_ref(base, fn, trim, flags, cb_data, entry);
@@ -495,10 +502,12 @@ static int do_for_each_ref_in_dirs(struct ref_dir *dir1,
if (cmp == 0) {
if ((e1->flag & REF_DIR) && (e2->flag & REF_DIR)) {
/* Both are directories; descend them in parallel. */
- sort_ref_dir(&e1->u.subdir);
- sort_ref_dir(&e2->u.subdir);
+ struct ref_dir *subdir1 = get_ref_dir(e1);
+ struct ref_dir *subdir2 = get_ref_dir(e2);
+ sort_ref_dir(subdir1);
+ sort_ref_dir(subdir2);
retval = do_for_each_ref_in_dirs(
- &e1->u.subdir, &e2->u.subdir,
+ subdir1, subdir2,
base, fn, trim, flags, cb_data);
i1++;
i2++;
@@ -521,9 +530,10 @@ static int do_for_each_ref_in_dirs(struct ref_dir *dir1,
i2++;
}
if (e->flag & REF_DIR) {
- sort_ref_dir(&e->u.subdir);
+ struct ref_dir *subdir = get_ref_dir(e);
+ sort_ref_dir(subdir);
retval = do_for_each_ref_in_dir(
- &e->u.subdir, 0,
+ subdir, 0,
base, fn, trim, flags, cb_data);
} else {
retval = do_one_ref(base, fn, trim, flags, cb_data, e);
@@ -751,11 +761,11 @@ static struct ref_dir *get_packed_refs(struct ref_cache *refs)
packed_refs_file = git_path("packed-refs");
f = fopen(packed_refs_file, "r");
if (f) {
- read_packed_refs(f, &refs->packed->u.subdir);
+ read_packed_refs(f, get_ref_dir(refs->packed));
fclose(f);
}
}
- return &refs->packed->u.subdir;
+ return get_ref_dir(refs->packed);
}
void add_packed_ref(const char *refname, const unsigned char *sha1)
@@ -809,7 +819,7 @@ static void read_loose_refs(struct ref_cache *refs, const char *dirname,
} else if (S_ISDIR(st.st_mode)) {
strbuf_addch(&refname, '/');
read_loose_refs(refs, refname.buf,
- &search_for_subdir(dir, refname.buf, 1)->u.subdir);
+ get_ref_dir(search_for_subdir(dir, refname.buf, 1)));
} else {
if (*refs->name) {
hashclr(sha1);
@@ -836,10 +846,10 @@ static struct ref_dir *get_loose_refs(struct ref_cache *refs)
if (!refs->loose) {
refs->loose = create_dir_entry("");
read_loose_refs(refs, "refs/",
- &search_for_subdir(&refs->loose->u.subdir,
- "refs/", 1)->u.subdir);
+ get_ref_dir(search_for_subdir(get_ref_dir(refs->loose),
+ "refs/", 1)));
}
- return &refs->loose->u.subdir;
+ return get_ref_dir(refs->loose);
}
/* We allow "recursive" symbolic refs. Only within reason, though */
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 15/18] search_for_subdir(): return (ref_dir *) instead of (ref_entry *)
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (13 preceding siblings ...)
2012-04-26 22:27 ` [PATCH v2 14/18] get_ref_dir(): add function for getting a ref_dir from a ref_entry mhagger
@ 2012-04-26 22:27 ` mhagger
2012-04-26 22:27 ` [PATCH v2 16/18] struct ref_dir: store a reference to the enclosing ref_cache mhagger
` (2 subsequent siblings)
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
That is what all the callers want, so give it to them.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/refs.c b/refs.c
index 2e71bfa..f6447e3 100644
--- a/refs.c
+++ b/refs.c
@@ -289,8 +289,8 @@ static struct ref_entry *search_ref_dir(struct ref_dir *dir, const char *refname
* directory if it is missing; otherwise, return NULL if the desired
* directory cannot be found.
*/
-static struct ref_entry *search_for_subdir(struct ref_dir *dir,
- const char *subdirname, int mkdir)
+static struct ref_dir *search_for_subdir(struct ref_dir *dir,
+ const char *subdirname, int mkdir)
{
struct ref_entry *entry = search_ref_dir(dir, subdirname);
if (!entry) {
@@ -299,8 +299,7 @@ static struct ref_entry *search_for_subdir(struct ref_dir *dir,
entry = create_dir_entry(subdirname);
add_entry_to_dir(dir, entry);
}
- assert(entry->flag & REF_DIR);
- return entry;
+ return get_ref_dir(entry);
}
/*
@@ -319,14 +318,14 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir,
const char *slash;
strbuf_init(&dirname, PATH_MAX);
for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {
- struct ref_entry *entry;
+ struct ref_dir *subdir;
strbuf_add(&dirname,
refname + dirname.len,
(slash + 1) - (refname + dirname.len));
- entry = search_for_subdir(dir, dirname.buf, mkdir);
- if (!entry)
+ subdir = search_for_subdir(dir, dirname.buf, mkdir);
+ if (!subdir)
break;
- dir = get_ref_dir(entry);
+ dir = subdir;
}
strbuf_release(&dirname);
@@ -819,7 +818,7 @@ static void read_loose_refs(struct ref_cache *refs, const char *dirname,
} else if (S_ISDIR(st.st_mode)) {
strbuf_addch(&refname, '/');
read_loose_refs(refs, refname.buf,
- get_ref_dir(search_for_subdir(dir, refname.buf, 1)));
+ search_for_subdir(dir, refname.buf, 1));
} else {
if (*refs->name) {
hashclr(sha1);
@@ -846,8 +845,8 @@ static struct ref_dir *get_loose_refs(struct ref_cache *refs)
if (!refs->loose) {
refs->loose = create_dir_entry("");
read_loose_refs(refs, "refs/",
- get_ref_dir(search_for_subdir(get_ref_dir(refs->loose),
- "refs/", 1)));
+ search_for_subdir(get_ref_dir(refs->loose),
+ "refs/", 1));
}
return get_ref_dir(refs->loose);
}
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 16/18] struct ref_dir: store a reference to the enclosing ref_cache
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (14 preceding siblings ...)
2012-04-26 22:27 ` [PATCH v2 15/18] search_for_subdir(): return (ref_dir *) instead of (ref_entry *) mhagger
@ 2012-04-26 22:27 ` mhagger
2012-04-26 22:27 ` [PATCH v2 17/18] read_loose_refs(): eliminate ref_cache argument mhagger
2012-04-26 22:27 ` [PATCH v2 18/18] refs: read loose references lazily mhagger
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
This means that a directory ref_entry contains all of the information
needed by read_loose_refs().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index f6447e3..110e801 100644
--- a/refs.c
+++ b/refs.c
@@ -106,6 +106,8 @@ struct ref_value {
unsigned char peeled[20];
};
+struct ref_cache;
+
struct ref_dir {
int nr, alloc;
@@ -117,6 +119,9 @@ struct ref_dir {
*/
int sorted;
+ /* A pointer to the ref_cache that contains this ref_dir. */
+ struct ref_cache *ref_cache;
+
struct ref_entry **entries;
};
@@ -234,12 +239,14 @@ static void clear_ref_dir(struct ref_dir *dir)
* dirname is the name of the directory with a trailing slash (e.g.,
* "refs/heads/") or "" for the top-level directory.
*/
-static struct ref_entry *create_dir_entry(const char *dirname)
+static struct ref_entry *create_dir_entry(struct ref_cache *ref_cache,
+ const char *dirname)
{
struct ref_entry *direntry;
int len = strlen(dirname);
direntry = xcalloc(1, sizeof(struct ref_entry) + len + 1);
memcpy(direntry->name, dirname, len + 1);
+ direntry->u.subdir.ref_cache = ref_cache;
direntry->flag = REF_DIR;
return direntry;
}
@@ -296,7 +303,7 @@ static struct ref_dir *search_for_subdir(struct ref_dir *dir,
if (!entry) {
if (!mkdir)
return NULL;
- entry = create_dir_entry(subdirname);
+ entry = create_dir_entry(dir->ref_cache, subdirname);
add_entry_to_dir(dir, entry);
}
return get_ref_dir(entry);
@@ -753,7 +760,7 @@ static struct ref_dir *get_packed_refs(struct ref_cache *refs)
const char *packed_refs_file;
FILE *f;
- refs->packed = create_dir_entry("");
+ refs->packed = create_dir_entry(refs, "");
if (*refs->name)
packed_refs_file = git_path_submodule(refs->name, "packed-refs");
else
@@ -843,7 +850,7 @@ static void read_loose_refs(struct ref_cache *refs, const char *dirname,
static struct ref_dir *get_loose_refs(struct ref_cache *refs)
{
if (!refs->loose) {
- refs->loose = create_dir_entry("");
+ refs->loose = create_dir_entry(refs, "");
read_loose_refs(refs, "refs/",
search_for_subdir(get_ref_dir(refs->loose),
"refs/", 1));
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 17/18] read_loose_refs(): eliminate ref_cache argument
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (15 preceding siblings ...)
2012-04-26 22:27 ` [PATCH v2 16/18] struct ref_dir: store a reference to the enclosing ref_cache mhagger
@ 2012-04-26 22:27 ` mhagger
2012-04-26 22:27 ` [PATCH v2 18/18] refs: read loose references lazily mhagger
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
The ref_cache can now be read from the ref_dir.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 110e801..a0c687f 100644
--- a/refs.c
+++ b/refs.c
@@ -785,9 +785,9 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)
* dirname must end with '/'. dir must be the directory entry
* corresponding to dirname.
*/
-static void read_loose_refs(struct ref_cache *refs, const char *dirname,
- struct ref_dir *dir)
+static void read_loose_refs(const char *dirname, struct ref_dir *dir)
{
+ struct ref_cache *refs = dir->ref_cache;
DIR *d;
const char *path;
struct dirent *de;
@@ -824,7 +824,7 @@ static void read_loose_refs(struct ref_cache *refs, const char *dirname,
/* Silently ignore. */
} else if (S_ISDIR(st.st_mode)) {
strbuf_addch(&refname, '/');
- read_loose_refs(refs, refname.buf,
+ read_loose_refs(refname.buf,
search_for_subdir(dir, refname.buf, 1));
} else {
if (*refs->name) {
@@ -851,7 +851,7 @@ static struct ref_dir *get_loose_refs(struct ref_cache *refs)
{
if (!refs->loose) {
refs->loose = create_dir_entry(refs, "");
- read_loose_refs(refs, "refs/",
+ read_loose_refs("refs/",
search_for_subdir(get_ref_dir(refs->loose),
"refs/", 1));
}
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 18/18] refs: read loose references lazily
2012-04-26 22:26 [PATCH v2 00/18] Read loose references lazily mhagger
` (16 preceding siblings ...)
2012-04-26 22:27 ` [PATCH v2 17/18] read_loose_refs(): eliminate ref_cache argument mhagger
@ 2012-04-26 22:27 ` mhagger
17 siblings, 0 replies; 26+ messages in thread
From: mhagger @ 2012-04-26 22:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jakub Narebski, Heiko Voigt, Johan Herland,
Christian Couder, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Instead of reading the whole directory of loose references the first
time any are needed, only read them on demand, one directory at a
time.
Use a new ref_entry flag bit REF_INCOMPLETE to indicate that the entry
represents a REF_DIR that hasn't been read yet. Whenever any entries
from such a directory are needed, read all of the loose references
from that directory.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 97 insertions(+), 30 deletions(-)
diff --git a/refs.c b/refs.c
index a0c687f..27ce968 100644
--- a/refs.c
+++ b/refs.c
@@ -101,6 +101,12 @@ int check_refname_format(const char *refname, int flags)
struct ref_entry;
+/*
+ * Information used (along with the information in ref_entry) to
+ * describe a single cached reference. This data structure only
+ * occurs embedded in a union in struct ref_entry, and only when
+ * (ref_entry->flag & REF_DIR) is zero.
+ */
struct ref_value {
unsigned char sha1[20];
unsigned char peeled[20];
@@ -108,6 +114,32 @@ struct ref_value {
struct ref_cache;
+/*
+ * Information used (along with the information in ref_entry) to
+ * describe a level in the hierarchy of references. This data
+ * structure only occurs embedded in a union in struct ref_entry, and
+ * only when (ref_entry.flag & REF_DIR) is set. In that case,
+ * (ref_entry.flag & REF_INCOMPLETE) determines whether the references
+ * in the directory have already been read:
+ *
+ * (ref_entry.flag & REF_INCOMPLETE) unset -- a directory of loose
+ * or packed references, already read.
+ *
+ * (ref_entry.flag & REF_INCOMPLETE) set -- a directory of loose
+ * references that hasn't been read yet (nor has any of its
+ * subdirectories).
+ *
+ * Entries within a directory are stored within a growable array of
+ * pointers to ref_entries (entries, nr, alloc). Entries 0 <= i <
+ * sorted are sorted by their component name in strcmp() order and the
+ * remaining entries are unsorted.
+ *
+ * Loose references are read lazily, one directory at a time. When a
+ * directory of loose references is read, then all of the references
+ * in that directory are stored, and REF_INCOMPLETE stubs are created
+ * for any subdirectories, but the subdirectories themselves are not
+ * read. The reading is triggered by get_ref_dir().
+ */
struct ref_dir {
int nr, alloc;
@@ -127,19 +159,33 @@ struct ref_dir {
/* ISSYMREF=0x01, ISPACKED=0x02, and ISBROKEN=0x04 are public interfaces */
#define REF_KNOWS_PEELED 0x08
+
+/* ref_entry represents a directory of references */
#define REF_DIR 0x10
/*
+ * Entry has not yet been read from disk (used only for REF_DIR
+ * entries representing loose references)
+ */
+#define REF_INCOMPLETE 0x20
+
+/*
* A ref_entry represents either a reference or a "subdirectory" of
- * references. Each directory in the reference namespace is
- * represented by a ref_entry with (flags & REF_DIR) set and
- * containing a subdir member that holds the entries in that
- * directory. References are represented by a ref_entry with (flags &
- * REF_DIR) unset and a value member that describes the reference's
- * value. The flag member is at the ref_entry level, but it is also
- * needed to interpret the contents of the value field (in other
- * words, a ref_value object is not very much use without the
- * enclosing ref_entry).
+ * references.
+ *
+ * Each directory in the reference namespace is represented by a
+ * ref_entry with (flags & REF_DIR) set and containing a subdir member
+ * that holds the entries in that directory that have been read so
+ * far. If (flags & REF_INCOMPLETE) is set, then the directory and
+ * its subdirectories haven't been read yet. REF_INCOMPLETE is only
+ * used for loose reference directories.
+ *
+ * References are represented by a ref_entry with (flags & REF_DIR)
+ * unset and a value member that describes the reference's value. The
+ * flag member is at the ref_entry level, but it is also needed to
+ * interpret the contents of the value field (in other words, a
+ * ref_value object is not very much use without the enclosing
+ * ref_entry).
*
* Reference names cannot end with slash and directories' names are
* always stored with a trailing slash (except for the top-level
@@ -176,10 +222,18 @@ struct ref_entry {
char name[FLEX_ARRAY];
};
+static void read_loose_refs(const char *dirname, struct ref_dir *dir);
+
static struct ref_dir *get_ref_dir(struct ref_entry *entry)
{
+ struct ref_dir *dir;
assert(entry->flag & REF_DIR);
- return &entry->u.subdir;
+ dir = &entry->u.subdir;
+ if (entry->flag & REF_INCOMPLETE) {
+ read_loose_refs(entry->name, dir);
+ entry->flag &= ~REF_INCOMPLETE;
+ }
+ return dir;
}
static struct ref_entry *create_ref_entry(const char *refname,
@@ -237,17 +291,17 @@ static void clear_ref_dir(struct ref_dir *dir)
/*
* Create a struct ref_entry object for the specified dirname.
* dirname is the name of the directory with a trailing slash (e.g.,
- * "refs/heads/") or "" for the top-level directory.
+ * "refs/heads/") or "" for the top-level directory.
*/
static struct ref_entry *create_dir_entry(struct ref_cache *ref_cache,
- const char *dirname)
+ const char *dirname, int incomplete)
{
struct ref_entry *direntry;
int len = strlen(dirname);
direntry = xcalloc(1, sizeof(struct ref_entry) + len + 1);
memcpy(direntry->name, dirname, len + 1);
direntry->u.subdir.ref_cache = ref_cache;
- direntry->flag = REF_DIR;
+ direntry->flag = REF_DIR | (incomplete ? REF_INCOMPLETE : 0);
return direntry;
}
@@ -263,7 +317,7 @@ static void sort_ref_dir(struct ref_dir *dir);
/*
* Return the entry with the given refname from the ref_dir
* (non-recursively), sorting dir if necessary. Return NULL if no
- * such entry is found.
+ * such entry is found. dir must already be complete.
*/
static struct ref_entry *search_ref_dir(struct ref_dir *dir, const char *refname)
{
@@ -294,7 +348,7 @@ static struct ref_entry *search_ref_dir(struct ref_dir *dir, const char *refname
* recursing). Sort dir if necessary. subdirname must be a directory
* name (i.e., end in '/'). If mkdir is set, then create the
* directory if it is missing; otherwise, return NULL if the desired
- * directory cannot be found.
+ * directory cannot be found. dir must already be complete.
*/
static struct ref_dir *search_for_subdir(struct ref_dir *dir,
const char *subdirname, int mkdir)
@@ -303,7 +357,13 @@ static struct ref_dir *search_for_subdir(struct ref_dir *dir,
if (!entry) {
if (!mkdir)
return NULL;
- entry = create_dir_entry(dir->ref_cache, subdirname);
+ /*
+ * Since dir is complete, the absence of a subdir
+ * means that the subdir really doesn't exist;
+ * therefore, create an empty record for it but mark
+ * the record complete.
+ */
+ entry = create_dir_entry(dir->ref_cache, subdirname, 0);
add_entry_to_dir(dir, entry);
}
return get_ref_dir(entry);
@@ -313,10 +373,10 @@ static struct ref_dir *search_for_subdir(struct ref_dir *dir,
* If refname is a reference name, find the ref_dir within the dir
* tree that should hold refname. If refname is a directory name
* (i.e., ends in '/'), then return that ref_dir itself. dir must
- * represent the top-level directory. Sort ref_dirs and recurse into
- * subdirectories as necessary. If mkdir is set, then create any
- * missing directories; otherwise, return NULL if the desired
- * directory cannot be found.
+ * represent the top-level directory and must already be complete.
+ * Sort ref_dirs and recurse into subdirectories as necessary. If
+ * mkdir is set, then create any missing directories; otherwise,
+ * return NULL if the desired directory cannot be found.
*/
static struct ref_dir *find_containing_dir(struct ref_dir *dir,
const char *refname, int mkdir)
@@ -760,7 +820,7 @@ static struct ref_dir *get_packed_refs(struct ref_cache *refs)
const char *packed_refs_file;
FILE *f;
- refs->packed = create_dir_entry(refs, "");
+ refs->packed = create_dir_entry(refs, "", 0);
if (*refs->name)
packed_refs_file = git_path_submodule(refs->name, "packed-refs");
else
@@ -781,9 +841,9 @@ void add_packed_ref(const char *refname, const unsigned char *sha1)
}
/*
- * Read the loose references for refs from the namespace dirname.
- * dirname must end with '/'. dir must be the directory entry
- * corresponding to dirname.
+ * Read the loose references from the namespace dirname into dir
+ * (without recursing). dirname must end with '/'. dir must be the
+ * directory entry corresponding to dirname.
*/
static void read_loose_refs(const char *dirname, struct ref_dir *dir)
{
@@ -824,8 +884,8 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
/* Silently ignore. */
} else if (S_ISDIR(st.st_mode)) {
strbuf_addch(&refname, '/');
- read_loose_refs(refname.buf,
- search_for_subdir(dir, refname.buf, 1));
+ add_entry_to_dir(dir,
+ create_dir_entry(refs, refname.buf, 1));
} else {
if (*refs->name) {
hashclr(sha1);
@@ -850,10 +910,17 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
static struct ref_dir *get_loose_refs(struct ref_cache *refs)
{
if (!refs->loose) {
- refs->loose = create_dir_entry(refs, "");
- read_loose_refs("refs/",
- search_for_subdir(get_ref_dir(refs->loose),
- "refs/", 1));
+ /*
+ * Mark the top-level directory complete because we
+ * are about to read the only subdirectory that can
+ * hold references:
+ */
+ refs->loose = create_dir_entry(refs, "", 0);
+ /*
+ * Create an incomplete entry for "refs/":
+ */
+ add_entry_to_dir(get_ref_dir(refs->loose),
+ create_dir_entry(refs, "refs/", 1));
}
return get_ref_dir(refs->loose);
}
--
1.7.10
^ permalink raw reply related [flat|nested] 26+ messages in thread