| Message ID | 67d858ec59e1c62d5e227e85e117b3cfcb3dda2a.1640199396.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 049cdbb059c28051fb96e1aba56558acf4b597a2 |
| Headers | show |
| Series | Reftable coverity fixes | expand |
"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Han-Wen Nienhuys <hanwen@google.com> > > Spotted by Coverity. > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > --- > 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.
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; switch (a->value_type) {