From: Junio C Hamano <gitster@pobox•com>
To: Wang Hui <Hui.Wang@windriver•com>
Cc: <git@vger•kernel.org>, <tali@admingilde•org>
Subject: Re: [PATCH v2 1/5] sha1_file cleanup: remove redundant variable check
Date: Tue, 06 Sep 2011 09:59:21 -0700 [thread overview]
Message-ID: <7vaaaha9p2.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1315304645-12009-2-git-send-email-Hui.Wang@windriver.com> (Wang Hui's message of "Tue, 6 Sep 2011 18:24:01 +0800")
Wang Hui <Hui.Wang@windriver•com> writes:
> 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] = '/';
next prev parent reply other threads:[~2011-09-06 16:59 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-06 10:24 [PATCH v2 0/5] sha1_file: remove only current repository can have relative path limitation Wang Hui
2011-09-06 10:24 ` [PATCH v2 1/5] sha1_file cleanup: remove redundant variable check Wang Hui
2011-09-06 10:24 ` [PATCH v2 2/5] sha1_file: remove a buggy value setting Wang Hui
2011-09-06 10:24 ` [PATCH v2 3/5] sha1_file: improve directories comparison method Wang Hui
2011-09-06 10:24 ` [PATCH v2 4/5] sha1_file: remove relative entries limitation Wang Hui
2011-09-06 10:24 ` [PATCH v2 5/5] t5710: add testcase for multi-level relative alternates Wang Hui
2011-09-06 16:32 ` [PATCH v2 3/5] sha1_file: improve directories comparison method Junio C Hamano
2011-09-06 16:26 ` [PATCH v2 2/5] sha1_file: remove a buggy value setting Junio C Hamano
2011-09-07 9:55 ` wanghui
2011-09-06 16:59 ` Junio C Hamano [this message]
2011-09-07 10:24 ` [PATCH v2 1/5] sha1_file cleanup: remove redundant variable check wanghui
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vaaaha9p2.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox$(echo .)com \
--cc=Hui.Wang@windriver$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=tali@admingilde$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox