Junio C Hamano wrote:
Wang Hui <Hui.Wang@xxxxxxxxxxxxx> writes:
From: Hui Wang <Hui.Wang@xxxxxxxxxxxxx>
This variable check is always true, so it is redundant and need to be
removed.
We can't remove the init value for this variable, since removing
it will introduce building warning:
'base_len' may be used uninitialized in this function.
If we are into cleaning things up, we should instead notice and say "yuck"
to the repeated "is entry is absolute and relative base is given" check.
Wouldn't something like this makes things easier to follow and also avoids
the "when does the path normalized and made absolute" issue?
Completely untested and I may have off-by-one errors and such, but I think
you would get the idea...
Yes, i got the idea, and some errors are pointed out below. I will
prepare a V3 patch basing on it.
sha1_file.c | 29 ++++++++++++-----------------
1 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index e002056..26aa3be 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -248,27 +248,22 @@ static int link_alt_odb_entry(const char * entry, int len, const char * relative
const char *objdir = get_object_directory();
struct alternate_object_database *ent;
struct alternate_object_database *alt;
- /* 43 = 40-byte + 2 '/' + terminating NUL */
- int pfxlen = len;
- int entlen = pfxlen + 43;
- int base_len = -1;
+ int pfxlen, entlen;
+ struct strbuf pathbuf = STRBUF_INIT;
if (!is_absolute_path(entry) && relative_base) {
- /* Relative alt-odb */
- if (base_len < 0)
- base_len = strlen(relative_base) + 1;
- entlen += base_len;
- pfxlen += base_len;
+ strbuf_addstr(&pathbuf, relative_base);
s/relative_base/real_path(relative_base)/ is better, since
normalize_path_copy is not good at handle relative path. e.g. ".
./objects/../../test2/objects" will be normalized to "objects"
+ strbuf_addch(&pathbuf, '/');
}
- ent = xmalloc(sizeof(*ent) + entlen);
+ strbuf_add(&pathbuf, entry, len);
+ normalize_path_copy(pathbuf.buf, pathbuf.buf);
if pathbuf.buf[strlen(pathbuf.buf)] is '/', remove it. this can avoid to
get a wrong result when comparing "/a/b" with "/a/b/".
+ strbuf_setlen(&pathbuf, strlen(pathbuf.buf));
above line can be removed, since we will release pathbuf soon.
- if (!is_absolute_path(entry) && relative_base) {
- memcpy(ent->base, relative_base, base_len - 1);
- ent->base[base_len - 1] = '/';
- memcpy(ent->base + base_len, entry, len);
- }
- else
- memcpy(ent->base, entry, pfxlen);
+ pfxlen = pathbuf.len;
+ 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] = '/';
--
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