Adam Spiers <git@xxxxxxxxxxxxxx> writes:
> Fix a corner case where check-ignore would segfault when run with the
> '.' argument from the top level of a repository, due to prefix_path()
> converting '.' into the empty string.
The description does not match what I understand is happening from
the original report, though. The above is more like this, no?
When check-ignore is run with the '.' argument from the top level of
a repository, it fed an empty string to hash_name() in name-hash.c
and caused a segfault, as the function kept reading forever past the
end of the string.
A point to note is that it is not cleaer why it is a corner case to
ask about a pathspec ".". It is a valid question "Is the whole tree
ignored by default?", isn't it?
> It doesn't make much sense to
> call check-ignore from the top level with '.' as a parameter, since
> the top-level directory would never typically be ignored,
And this sounds like a really bad excuse. If it were "it does not
make *any* sense ... because the top level is *never* ignored", then
the patch is a perfectly fine optimization that happens to work
around the problem, but the use of "much" and "typically" is a sure
sign that the design of the fix is iffy. It also shows that the
patch is not a fix, but is sweeping the problem under the rug, if
there were a valid use case to set the top level to be ignored.
I wonder what would happen if we removed that "&& path[0]" check
from the caller, not add the "assume the top is never ignored"
workaround, and do something like this to the location that causes
segv, so that it can give an answer when asked to hash an empty
string?
Does the callchain that goes down to this function have other places
that assume they will never see an empty string, like this function
does, which I _think_ is the real issue?
name-hash.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/name-hash.c b/name-hash.c
index d8d25c2..942c459 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -24,11 +24,11 @@ static unsigned int hash_name(const char *name, int namelen)
{
unsigned int hash = 0x123;
- do {
+ while (namelen--) {
unsigned char c = *name++;
c = icase_hash(c);
hash = hash*101 + c;
- } while (--namelen);
+ }
return hash;
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html