Re: [PATCH v5] fetch-pack: always allow fetching of literal SHA1s
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- May 11, 2017, 22:46 UTC
- Message-ID
- <20170511224639.GC21723@aiede.svl.corp.google.com>
- In-Reply-To
- <20170511223054.25239-1-jonathantanmy@google.com>
Jonathan Tan wrote:
[...]
Show 25 quoted lines
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -15,6 +15,7 @@
> #include "version.h"
> #include "prio-queue.h"
> #include "sha1-array.h"
> +#include "oidset.h"
>
> static int transfer_unpack_limit = -1;
> static int fetch_unpack_limit = -1;
> @@ -592,13 +593,32 @@ static void mark_recent_complete_commits(struct fetch_pack_args *args,
> }
> }
>
> +static void add_refs_to_oidset(struct oidset *oids, struct ref *refs)
> +{
> + for (; refs; refs = refs->next)
> + oidset_insert(oids, &refs->old_oid);
> +}
> +
> +static int tip_oids_contain(struct oidset *tip_oids,
> + struct ref *unmatched, struct ref *newlist,
> + const struct object_id *id)
> +{
> + if (!tip_oids->map.cmpfn) {This feels like a layering violation. Could it be e.g. a static inline function oidset_is_initialized in oidset.h?
Show 5 quoted lines
> + add_refs_to_oidset(tip_oids, unmatched); > + add_refs_to_oidset(tip_oids, newlist); > + } > + return oidset_contains(tip_oids, id); > +}
The rest looks good.
With or without that change, Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks for your patient work.
diff --git i/fetch-pack.c w/fetch-pack.c index 9dd430a65a..0394580434 100644 --- i/fetch-pack.c +++ w/fetch-pack.c @@ -603,7 +603,7 @@ static int tip_oids_contain(struct oidset *tip_oids, struct ref *unmatched, struct ref *newlist, const struct object_id *id) { - if (!tip_oids->map.cmpfn) { + if (!oidset_initialized(tip_oids)) { add_refs_to_oidset(tip_oids, unmatched); add_refs_to_oidset(tip_oids, newlist); } diff --git i/oidset.c w/oidset.c index ac169f05d3..f2a6753b8a 100644 --- i/oidset.c +++ w/oidset.c @@ -18,7 +18,7 @@ int oidset_contains(const struct oidset *set, const struct object_id *oid) { struct hashmap_entry key; - if (!set->map.cmpfn) + if (!oidset_initialized(set)) return 0; hashmap_entry_init(&key, sha1hash(oid->hash)); @@ -29,7 +29,7 @@ int oidset_insert(struct oidset *set, const struct object_id *oid) { struct oidset_entry *entry; - if (!set->map.cmpfn) + if (!oidset_initialized(set)) hashmap_init(&set->map, oidset_hashcmp, 0); if (oidset_contains(set, oid)) diff --git i/oidset.h w/oidset.h index b7eaab5b88..2e7d889770 100644 --- i/oidset.h +++ w/oidset.h @@ -22,6 +22,16 @@ struct oidset { #define OIDSET_INIT { { NULL } } +/** + * Returns true iff "set" has been initialized (for example by inserting + * an entry). An oidset is considered uninitialized if it hasn't had any + * oids inserted since it was last cleared. + */ +static inline int oidset_initialized(const struct oidset *set) +{ + return set->map.cmpfn ? 1 : 0; +} + /** * Returns true iff `set` contains `oid`. */