Am 30.06.2014 15:43, schrieb Jeff King: > On Sun, Jun 29, 2014 at 07:56:25AM +0200, René Scharfe wrote: > >> Avoid overrunning the existing pack name (p->pack_name, a C string) in >> the case that the new path is longer by using strncmp instead of memcmp >> for comparing. While at it, replace the magic constant 4 with a >> strlen call to document its meaning. > > An unwritten assumption with this code is that we have "foo.idx" and we > want to make sure that we are matching "foo.pack" from the existing pack > list. Both before and after your patch, I think we would match > "foobar.pack". Yes, indeed. > It's probably not a big deal, as we don't expect random > junk in the pack directory, but I wonder if it would be better to be > explicit, like: <snip> Here's a simpler approach: -- >8 -- Subject: [PATCH v2 3/2] sha1_file: more precise packname matching Consider the full length of the already loaded pack names when checking for duplicates. Signed-off-by: Rene Scharfe <l.s.r@xxxxxx> --- sha1_file.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index b7ad6c1..a13fbbd 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1206,10 +1206,12 @@ static void prepare_packed_git_one(char *objdir, int local) strbuf_addstr(&path, de->d_name); if (has_extension(de->d_name, ".idx")) { + size_t len = path.len - strlen(".idx"); + /* Don't reopen a pack we already have. */ for (p = packed_git; p; p = p->next) { - if (!strncmp(path.buf, p->pack_name, - path.len - strlen(".idx"))) + if (!strncmp(p->pack_name, path.buf, len) && + !strcmp(p->pack_name + len, ".pack")) break; } if (p == NULL && -- 2.0.0 -- 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