From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 07 Jun 2024 16:59:13 GMT
Subject: Re: [PATCH v2] apply: add unit tests for parse_range
Message-ID: <xmqqo78celjy.fsf@gitster.g>
In-Reply-To: <b7eca313-9ea8-4132-ba1d-ed9236e07095@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> 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.