On Thu, Feb 8, 2024 at 11:48 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > René Scharfe <l.s.r@xxxxxx> writes: > > > But anyway: If NULs are of no concern and we currently end parsing when > > we see one in all cases, why do we need a _mem function at all? The > > original version of the function, find_commit_header(), should suffice. > > check_nonce() could be run against the NUL-terminated sigcheck.payload > > and check_cert_push_options() parses an entire strbuf, so there is no > > risk of out-of-bounds access. > > If I recall correctly, the caller that does not pass strlen() as the > payload length gives a length that is shorter than the buffer, i.e. > "stop the parsing here, do not get confused into thinking the > garbage after this point contains useful payload" was the reason why > we have a separate "len". > I just rediscovered that. I think this probably should be something that caller (check_nonce) implements, then. Having a _mem function implies to me (though I'm very new to this codebase) that it supports embedded NULs, but that's not what's happening here.