[PATCH] link_alt_odb_entries: make empty input a noop
- From
Jeff King <peff@peff.net>
- Date
- Nov 12, 2017, 10:27 UTC
- Message-ID
- <20171112102739.6xtnnsmtabhnhrm5@sigill.intra.peff.net>
- In-Reply-To
- <xmqqd14pef5q.fsf@gitster.mtv.corp.google.com>
On Sat, Nov 11, 2017 at 11:13:21AM +0900, Junio C Hamano wrote:
Show 29 quoted lines
> Jeff King <peff@peff.net> writes: > > > So totally orthogonal to your bug, I wonder if we ought to be doing: > > > > diff --git a/sha1_file.c b/sha1_file.c > > index 057262d46e..0b76233aa7 100644 > > --- a/sha1_file.c > > +++ b/sha1_file.c > > @@ -530,11 +530,11 @@ void prepare_alt_odb(void) > > if (alt_odb_tail) > > return; > > > > - alt = getenv(ALTERNATE_DB_ENVIRONMENT); > > - if (!alt) alt = ""; > > - > > alt_odb_tail = &alt_odb_list; > > - link_alt_odb_entries(alt, strlen(alt), PATH_SEP, NULL, 0); > > + > > + alt = getenv(ALTERNATE_DB_ENVIRONMENT); > > + if (alt) > > + link_alt_odb_entries(alt, strlen(alt), PATH_SEP, NULL, 0); > > > > read_info_alternates(get_object_directory(), 0); > > } > > > > to avoid hitting link_alt_odb_entries() at all when there are no > > entries. > > Sounds sane.
Here it is as a real patch. I actually bumped the check into the function itself, since it keeps the logic all in one place. And as a bonus, we save work if you truly have an empty environment variable or info/alternates file, though I don't expect those are very common. :)
I also rebased on top of dc732bd5cb (read_info_alternates: read contents into strbuf, 2017-09-19), which had a trivial textual conflict.
This should make Joey's immediate pain go away, though only by papering it over. I tend to agree that we shouldn't be looking at $PWD at all here.
-- >8 -- Subject: [PATCH] link_alt_odb_entries: make empty input a noop
If an empty string is passed to link_alt_odb_entries(), our loop finds no entries and we link nothing. But we still do some preparatory work to normalize the object directory path, even though we'll never look at the result. This triggers in basically every git process, since we feed the usually-empty ALTERNATE_DB_ENVIRONMENT to the function.
Let's detect early that there's nothing to do and return. While we're at it, let's treat NULL the same as an empty string as a favor to our callers. That saves prepare_alt_odb() from having to cover this case.
Signed-off-by: Jeff King <peff@peff.net> --- sha1_file.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sha1_file.c b/sha1_file.c index d708981376..8a7c6b7eba 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -404,6 +404,9 @@ static void link_alt_odb_entries(const char *alt, int sep, struct strbuf objdirbuf = STRBUF_INIT; struct strbuf entry = STRBUF_INIT; + if (!alt || !*alt) + return; + if (depth > 5) { error("%s: ignoring alternate object stores, nesting too deep.", relative_base); @@ -604,7 +607,6 @@ void prepare_alt_odb(void) return; alt = getenv(ALTERNATE_DB_ENVIRONMENT); - if (!alt) alt = ""; alt_odb_tail = &alt_odb_list; link_alt_odb_entries(alt, PATH_SEP, NULL, 0);
-- 2.15.0.413.g6cc52d366b