Subject: Re: [PATCH v2] apply: add unit tests for parse_range

From: Junio C Hamano <[email-masked]>
Date: Fri, 07 Jun 2024 09:59:13 -0700

Phillip Wood <[email-masked]> writes:

> Thanks for re-rolling, I've left a few comments below
> ...
> This is an example of why I don't think we should check p1 and p2 when
> we're expecting the parse to fail. Also please note we don't use "//"
> comments in the code base.
>
> There is a good range of failing non-digit inputs. It would be nice to
> see a test for a hunk header where the number is too large to
> parse. Ideally we'd also change the parsing code to return an error in
> that case rather than waiting to error out later on as the apply code
> does currently. To do that You'd need to explicitly set errno to zero
> before calling strtoul() and check it afterwards.

Thanks.  All points you raised make quite a lot of sense to me.