Re: [PATCH v2 1/5] sha1_file cleanup: remove redundant variable check
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 6, 2011, 16:59 UTC
- Message-ID
- <7vaaaha9p2.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1315304645-12009-2-git-send-email-Hui.Wang@windriver.com>
Wang Hui <Hui.Wang@windriver.com> writes:
Show 8 quoted lines
> From: Hui Wang <Hui.Wang@windriver.com> > > 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...
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); + strbuf_addch(&pathbuf, '/'); } - ent = xmalloc(sizeof(*ent) + entlen); + strbuf_add(&pathbuf, entry, len); + normalize_path_copy(pathbuf.buf, pathbuf.buf); + strbuf_setlen(&pathbuf, strlen(pathbuf.buf)); - 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] = '/';