From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Kyle J. McKay" <mackyle@gmail.com>,
Git mailing list <git@vger.kernel.org>
Subject: [PATCH 4/4] fetch-pack: remove dead assignment to ref->new_sha1
Date: Thu, 19 Mar 2015 16:39:36 -0400 [thread overview]
Message-ID: <20150319203935.GD7666@peff.net> (raw)
In-Reply-To: <20150319203126.GA31663@peff.net>
In everything_local(), we used to assign the current ref's value
found in ref->old_sha1 to ref->new_sha1 when we already have all the
necessary objects to complete the history leading to that
commit. This copying was broken at 49bb805e (Do not ask for
objects known to be complete., 2005-10-19) and ever since we
instead stuffed a random bytes in ref->new_sha1 here. No
code complained or failed due to this breakage.
It turns out that no code path that comes after this
assignment even looks at ref->new_sha1 at all.
- The only caller of everything_local(), do_fetch_pack(),
returns this list of refs, whose element has bogus
new_sha1 values, to its caller. It does not look at the
elements itself, but does pass them to find_common, which
looks only at the name and old_sha1 fields.
- The only caller of do_fetch_pack(), fetch_pack(), returns this
list to its caller. It does not look at the elements nor act on
them.
- One of the two callers of fetch_pack() is cmd_fetch_pack(), the
top-level that implements "git fetch-pack". The only thing it
looks at in the elements of the returned ref list is the old_sha1
and name fields.
- The other caller of fetch_pack() is fetch_refs_via_pack() in the
transport layer, which is a helper that implements "git fetch".
It only cares about whether the returned list is empty (i.e.
failed to fetch anything).
Just drop the bogus assignment, that is not even necessary. The
remote-tracking refs are updated based on a different list and not
using the ref list being manipulated by this code path; the caller
do_fetch_pack() created a copy of that real ref list and passed the
copy down to this function, and modifying the elements here does not
affect anything.
Noticed-by: Kyle J. McKay <mackyle@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
fetch-pack.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 84d5a66..48526aa 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -628,7 +628,6 @@ static int everything_local(struct fetch_pack_args *args,
for (retval = 1, ref = *refs; ref ; ref = ref->next) {
const unsigned char *remote = ref->old_sha1;
- unsigned char local[20];
struct object *o;
o = lookup_object(remote);
@@ -641,8 +640,6 @@ static int everything_local(struct fetch_pack_args *args,
ref->name);
continue;
}
-
- hashcpy(ref->new_sha1, local);
if (!args->verbose)
continue;
fprintf(stderr,
--
2.3.3.520.g3cfbb5d
prev parent reply other threads:[~2015-03-19 20:39 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-15 6:37 Bug in fetch-pack.c, please confirm Kyle J. McKay
2015-03-15 7:27 ` Junio C Hamano
2015-03-15 7:30 ` Junio C Hamano
2015-03-15 22:21 ` Kyle J. McKay
2015-03-16 1:13 ` Jeff King
2015-03-19 17:41 ` Junio C Hamano
2015-03-19 18:55 ` Jeff King
2015-03-19 19:01 ` Junio C Hamano
2015-03-19 20:31 ` Jeff King
2015-03-19 20:34 ` [PATCH 1/4] filter_ref: avoid overwriting ref->old_sha1 with garbage Jeff King
2015-03-19 21:09 ` Junio C Hamano
2015-03-19 20:37 ` [PATCH 2/4] filter_ref: make a copy of extra "sought" entries Jeff King
2015-03-19 20:38 ` [PATCH 3/4] fetch_refs_via_pack: free extra copy of refs Jeff King
2015-03-19 20:39 ` Jeff King [this message]
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=20150319203935.GD7666@peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mackyle@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).