Jamey Sharp <jamey@xxxxxxxxxxx> writes:
> Furthermore, for_each_ref and for_each_ref_submodule passed "refs/" but
> a length of 0, which caused do_for_each_ref to ignore the "refs/".
I had to read, stop, think for two days, until finally get to the point
that I _think_ I understand what you wanted to say.
As we use the same "trim" (meant to say "strip this many bytes from the
beginning of the full refname when calling the callback") to reject refs
outside the area we are interested in with the strncmp() at the beginning
of do_one_ref(), if do_for_each_ref() that is called by for_each_ref() fed
something outside "refs/" hierarchy to the function, the garbage ref that
is not a ref (as it is outside "refs/") will _not_ get filtered, which I
think is what you are trying to say by 'ignore the "refs/"'.
Which is technically a bug (we should be rejecting anything outside
"refs/", even when trim is set to 0) that dates as far back as e1e22e3
(Start handling references internally as a sorted in-memory list,
2006-09-11), but it didn't matter an iota because everything we read from
either loose or packed refs have "refs/" prefix.
Am I following your train of thought correctly so far?
> diff --git a/refs.c b/refs.c
> index e3c0511..60cebe6 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -584,7 +584,7 @@ int read_ref(const char *ref, unsigned char *sha1)
> static int do_one_ref(const char *base, each_ref_fn fn, int trim,
> int flags, void *cb_data, struct ref_list *entry)
> {
> - if (strncmp(base, entry->name, trim))
> + if (prefixcmp(entry->name, base))
> return 0;
>
> if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) {
> ...
> int for_each_ref(each_ref_fn fn, void *cb_data)
> {
> - return do_for_each_ref(NULL, "refs/", fn, 0, 0, cb_data);
> + return for_each_ref_in("", fn, cb_data);
> }
But then this looks like a bad way to fix that issue. It will be a
non-issue as long as do-for-each-ref will never give anything outside
"refs/", but once that happens (say, a contaminated .git/packed-refs
file), this will show whatever that is outside "refs/", i.e. the issue the
proposed commit log message claims to address, which is "... which caused
do_for_each_ref to ignore", is not fixed here at all.
Shouldn't you be passing prefix and trim the same way as we have always
done, but just fixing the strncmp() at the beginning of do_one_ref()?
--
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