On Wed, Feb 07, 2024 at 07:08:37PM -0800, Junio C Hamano wrote: > > FWIW, I would have gone the other way, and fixed sign_buffer_ssh(). Your > > solution here is future-proofing the tag code against other > > sign_buffer_*() functions behaving like ssh. But it is also leaving > > other sign_buffer() callers to introduce the same bug. > > > > Your documentation change at least makes that less likely. But given how > > much of our code uses the "negative is error" convention, I wouldn't be > > surprised to see it happen anyway. > > Yeah, but other callers are prepared to honor the current return > value convention used by gpg-interface, so "fixing" sign_buffer_ssh() > would not give us any future-proofing. It future-proofs against a hypothetical new sign_buffer() caller (just like your patch future-proofs against a hypothetical new signing backend). > We could do belt and suspenders by tightening the other callers to > only expect negative for errors (but then what should they do when > they receive non-zero positive? Should they BUG() out???) while > teaching sign_buffer_ssh() that our convention is to return negative > for an error, of course, but I am not sure if it that is worth it. I'm not sure that's worth it, since we'd only notice if the error triggered (so writing a test). -Peff
