Re: [PATCH 2/7] gpg-interface: make signature-prefix matching length-aware

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> That holds for six of the seven call sites: `commit.c`, `tag.c`,
> `builtin/fast-import.c`, `fmt-merge-msg.c`, and `log-tree.c` (twice).
> `builtin/receive-pack.c` is a minor wrinkle worth flagging: it passes
> `push_cert.buf + bogs` ("bogs" = "beginning_of_gpg_sig") and
> `push_cert.len - bogs`, an offset sub-buffer of `push_cert`, not that
> strbuf's own buf/len pair verbatim. It still ends on `push_cert`'s own
> terminating NUL, so the observation holds in spirit, but strictly the
> pattern is "ends at some strbuf's own NUL", which is more a matter of
> code-review convention across call sites than something
> `check_signature()`'s own signature guarantees.

Yes but the audit was "is slen our callers pass redundant?", and not
"does everybody pass strbuf and we are better off passing a pionter
to a strbuf?".  And the answer to the former question is "yes".

And I do not quite understand or agree with the logic here.

> ... Applying the same "audit
> today's callers and assume it holds" reasoning to `check_signature()` now
> risks reproducing that failure mode a second time.

What I was saying was to force all current *and* *future* callers to
pass NUL-terminated string by removing slen.

Having said all that, I think this falls into "once the code is
written (and more importantly, once it is reviewed, as that is a lot
more costly part of the development process for machine written
code), it is not worth going back and change it, as the difference
is not large enough either way."