On Tue, Jun 08, 2021 at 10:48:41PM +0200, Andrei Rybak wrote: > Existing checks for scissors characters using memcmp(3) never read past > the end of the line, because all substrings we are interested in are two > characters long, and the outer loop guarantees we have at least one > character. So at most we will look at the NUL. > > However, this is too subtle and may lead to bugs in code which copies > this behavior without realizing substring length requirement. So use > starts_with() instead, which will stop at NUL regardless of the length > of the prefix. Remove extra pair of parentheses while we are here. > > Helped-by: Jeff King <peff@xxxxxxxx> > Signed-off-by: Andrei Rybak <rybak.a.v@xxxxxxxxx> > --- > mailinfo.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > This patch was originally part of: > https://public-inbox.org/git/20190401215334.18678-1-rybak.a.v@xxxxxxxxx/ > I've finally gotten around to sending it on its own. Well, after seeing the date I don't feel too bad for not remembering it. :) I think this is an improvement in safety and readability, and worth applying. I'd also be fine going further and using skip_prefix(), which would let us drop the magic-number "2", though: - as Junio said in that earlier thread, we hope people aren't encouraged to add more versions of scissors here anyway). - I am not 100% sure that the increment of "c" in the conditional is not also relying on that "2", as well (i.e., it is incrementing one, and then the for-loop increments one). There's a non-trivial risk of introducing off-by-one errors or even more subtlety here. :) So I'm OK leaving that. Thanks for resurrecting this cleanup. -Peff
