# Re: \[PATCH v5 10/16\] reftable: handle null refnames in reftable\_ref\_record\_equal

- From: Junio C Hamano \<hidden\>
- Date: 2021-12-22 22:51:24 
- List: git
- Page: https://ratatoskr.run/git/2021/12/7916686
- Thread: https://ratatoskr.run/git/2021/12/7914155/t/2.md

```text
"Han-Wen Nienhuys via GitGitGadget" [ref] writes:

> From: Han-Wen Nienhuys <redacted>
>
> Spotted by Coverity.
>
> Signed-off-by: Han-Wen Nienhuys <redacted>
> ---
>  reftable/record.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/reftable/record.c b/reftable/record.c
> index 8536bd03aa9..8bbcbff1e69 100644
> --- a/reftable/record.c
> +++ b/reftable/record.c
> @@ -1154,9 +1154,11 @@ int reftable_ref_record_equal(struct reftable_ref_record *a,
>  			      struct reftable_ref_record *b, int hash_size)
>  {
>  	assert(hash_size > 0);
> -	if (!(0 == strcmp(a->refname, b->refname) &&
> -	      a->update_index == b->update_index &&
> -	      a->value_type == b->value_type))
> +	if (!null_streq(a->refname, b->refname))
> +		return 0;
> +
> +	if (a->update_index != b->update_index ||
> +	    a->value_type != b->value_type)
>  		return 0;

The original assumed that the .refname member of these two records
are filled, so strcmp() would have segfaulted if they were set to
NULL, which equates to an empty string.  I assume that this patch
is to fix that and change nothing else?

The original said: we'll make an early return with 0 unless
all three conditions hold true; (1) names are the same, (2)
update_index members are the same, and (3) value_type members
are the same.

We now say, "we return early with 0 if names are different", and "we
also return early with 0 if update-index or value-type members are
different".

I just found the splitting into two separate statements looked
different from the original, but they do mean the same, so it is OK.

In fact, I think the !(A && B && C) is much less readable than
(!A || !B || !C) in this case, so I am OK with the new version.

Looking good.  Thanks.

```