Re: [PATCH v3 1/1] sha1_file: normalize alt_odb path before comparing and storing
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:59
Wang Hui [off-list ref] writes:
From: Hui Wang <redacted> When it needs to compare and add an alt object path to the alt_odb_list, we normalize this path first since comparing normalized path is easy to get correct result. Use strbuf to replace some string operations, since it is cleaner and safer.
Thanks, will queue.
quoted hunk
diff --git a/sha1_file.c b/sha1_file.c index f7c3408..fa2484b 100644 --- a/sha1_file.c +++ b/sha1_file.c@@ -248,27 +248,27 @@ static int link_alt_odb_entry(const char * entry, int len, const char * relative... + /* Drop the last '/' from path can make memcmp more accurate */ + if (pathbuf.buf[pfxlen-1] == '/') + pfxlen -= 1;
By the way, I do not necessarily agree with the above comment. As long as
you consistently strip the trailing slashes from all directory paths, or
you consistently leave a single trailing slash after all directory paths,
you can get accurate comparison either way.
Side note: I tend to prefer keeping a single trailing slash when I
know what we are talking about is a directory in general, because
you do not have to worry about the corner case near the root.
Compare ('/' and '/bin/') vs ('/' and '/bin').
In this particular case, the real reason you want to remove the trailing
slash is that the invariants of ent->base[] demands it (after all, it
places another slash immediately after it), and making pathbuf.buf[] an
empty string (i.e. pfxlen == 0) would still be OK to represent an
alternate object store at the root level (this function assigns '/' at
ent->base[pfxlen] immediately before returning, and that '/' names the
root directory).
+ entlen = pfxlen + 43; /* '/' + 2 hex + '/' + 38 hex + NUL */ + ent = xmalloc(sizeof(*ent) + entlen); + memcpy(ent->base, pathbuf.buf, pfxlen); + strbuf_release(&pathbuf); ent->name = ent->base + pfxlen + 1; ent->base[pfxlen + 3] = '/';