Re: [PATCH v6 12/15] reftable: implement record equality generically
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jan 21, 2022, 11:52 UTC
- Message-ID
- <220121.86sfthjocg.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <6385e449ba714e3ce41f10276fc60e9757519492.1642691534.git.gitgitgadget@gmail.com>
On Thu, Jan 20 2022, Han-Wen Nienhuys via GitGitGadget wrote:
Show 21 quoted lines
> From: Han-Wen Nienhuys <hanwen@google.com> > > This simplifies unittests a little, and provides further coverage for > reftable_record_copy(). > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > --- > reftable/record.c | 57 +++++++++++++++++++++++++++++++++++++++++- > reftable/record.h | 5 +++- > reftable/record_test.c | 23 +++-------------- > 3 files changed, 63 insertions(+), 22 deletions(-) > > diff --git a/reftable/record.c b/reftable/record.c > index f7c77c51539..2a9e41a992e 100644 > --- a/reftable/record.c > +++ b/reftable/record.c > @@ -430,6 +430,15 @@ static int reftable_ref_record_is_deletion_void(const void *p) > (const struct reftable_ref_record *)p); > } > > +
stray extra newline being added here.
Show 35 quoted lines
> +static int reftable_ref_record_equal_void(const void *a,
> + const void *b, int hash_size)
> +{
> + struct reftable_ref_record *ra = (struct reftable_ref_record *) a;
> + struct reftable_ref_record *rb = (struct reftable_ref_record *) b;
> + return reftable_ref_record_equal(ra, rb, hash_size);
> +}
> +
> static struct reftable_record_vtable reftable_ref_record_vtable = {
> .key = &reftable_ref_record_key,
> .type = BLOCK_TYPE_REF,
> @@ -439,6 +448,7 @@ static struct reftable_record_vtable reftable_ref_record_vtable = {
> .decode = &reftable_ref_record_decode,
> .release = &reftable_ref_record_release_void,
> .is_deletion = &reftable_ref_record_is_deletion_void,
> + .equal = &reftable_ref_record_equal_void,
> };
>
> static void reftable_obj_record_key(const void *r, struct strbuf *dest)
> @@ -572,6 +582,25 @@ static int not_a_deletion(const void *p)
> return 0;
> }
>
> +static int reftable_obj_record_equal_void(const void *a, const void *b, int hash_size)
> +{
> + struct reftable_obj_record *ra = (struct reftable_obj_record *) a;
> + struct reftable_obj_record *rb = (struct reftable_obj_record *) b;
> +
> + if (ra->hash_prefix_len != rb->hash_prefix_len
> + || ra->offset_len != rb->offset_len)
> + return 0;
> +
> + if (ra->hash_prefix_len &&
> + memcmp(ra->hash_prefix, rb->hash_prefix, ra->hash_prefix_len))
> + return 0;Similar to the memcpy() paranoia isn't this memcmp() paranoia? I.e. memcmp() returns 0 on a n=0, so we can lose the "ra->hash_prefix_len &&" here, no?
> + if (ra->offset_len && > + memcmp(ra->offsets, rb->offsets, ra->offset_len * sizeof(uint64_t))) > + return 0;
...and here, since 0 * sizeof() will just get us zero.