All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org,  Sergey Kosukhin <skosukhin@gmail.com>
Subject: Re: [PATCH] tag: fix sign_buffer() call to create a signed tag
Date: Wed, 07 Feb 2024 19:08:37 -0800	[thread overview]
Message-ID: <xmqq5xyzr6tm.fsf@gitster.g> (raw)
In-Reply-To: <20240208004757.GA1059751@coredump.intra.peff.net> (Jeff King's message of "Wed, 7 Feb 2024 19:47:57 -0500")

Jeff King <peff@peff.net> writes:

>>  * 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.

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.

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.

Thanks.


  reply	other threads:[~2024-02-08  3:08 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-07 18:46 [PATCH] tag: fix sign_buffer() call to create a signed tag Junio C Hamano
2024-02-08  0:47 ` Jeff King
2024-02-08  3:08   ` Junio C Hamano [this message]
2024-02-08  5:29     ` Junio C Hamano
2024-02-08 21:27       ` Jeff King
2024-02-08 20:26     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq5xyzr6tm.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=skosukhin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.