Michael Haggerty <mhagger@xxxxxxxxxxxx> writes: > So `ref_transaction_update()` *does* need to set or clear the `HAVE_NEW` > and `HAVE_OLD` bits as I sketched, to impedance-match between the two > conventions. OK, so ignoring HAVE_NEW/HAVE_OLD bits that the callers of ref_transaction_update() may set in flags, and having ref_transaction_update() compute these bits based on new/old_sha1 pointers from scratch, would be the right thing to do. IOW flags &= ~(REF_HAVE_NEW|REF_HAVE_OLD); if (new_sha1) flags |= REF_HAVE_NEW; if (old_sha1) flags |= REF_HAVE_OLD; and your earlier "Does the warning go away if you change the line to" does essentially the same thing. > It's a shame how much time we've wasted discussing this. Maybe the code > is trying to be too clever/efficient and needs a rethink. It might be the case, but I do not know what to blame is "the two conventions", an over-eager compiler, or a confused commenter on the thread (that's me), though ;-).
