On Wed, Feb 07, 2024 at 10:46:54AM -0800, Junio C Hamano wrote: > The command "git tag -s" internally calls sign_buffer() to make a > cryptographic signature using the chosen backend like GPG and SSH. > The internal helper functions used by "git tag" implementation seem > to use a "negative return values are errors, zero or positive return > values are not" convention, and there are places (e.g., verify_tag() > that calls gpg_verify_tag()) that these internal helper functions > translate return values that signal errors to conform to this > convention, but do_sign() that calls sign_buffer() forgets to do so. > > Fix it, so that a failed call to sign_buffer() that can return the > exit status from pipe_command() will not be overlooked. > > Reported-by: Sergey Kosukhin <skosukhin@xxxxxxxxx> > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > --- > > * We alternatively could fix individual sign_buffer() backend that > signals an error with a positive value (sign_buffer_ssh() in this > case) to return a negative value, but this would hopefully be > more future-proof. 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. -Peff
