Re: [PATCH v4 2/2] patch-id: replace `atoi()` with `strtol_i_updated()`

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

"Mohit Marathe via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  	q = p + 4;
>  	n = strspn(q, digits);
>  	if (q[n] == ',') {
>  		q += n + 1;

So, we saw "@@ -" and skipped over these four bytes, skipped the
digits from there, and found a comma.  

For "@@ -29,14 +30,18 @@", for example, our q is now "14 +30,18 @@"
as we have skipped over that comma after 29.

> -		*p_before = atoi(q);
> +		if (strtol_i_updated(q, 10, p_before, &endp) != 0)
> +			return 0;

We parse out 14 and store it to *p_before.  endp points at " +30..."
now.

>  		n = strspn(q, digits);
> +		if (endp != q + n)
> +			return 0;

Is this necessary?  By asking strtol_i_updated() where the number ended,
we already know endp without skipping the digits in q with strspn().
Shouldn't these three lines become more like

		n = endp - q;

instead?  

After all, we are not trying to find a bug in strtol_i_updated(),
which would be the only reason how this "return 0" would trigger.

>  	} else {
>  		*p_before = 1;
>  	}
> @@ -48,8 +53,11 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)
>  	n = strspn(r, digits);
>  	if (r[n] == ',') {
>  		r += n + 1;
> -		*p_after = atoi(r);
> +		if (strtol_i_updated(r, 10, p_after, &endp) != 0)
> +			return 0;
>  		n = strspn(r, digits);
> +		if (endp != r + n)
> +			return 0;

Likewise.

>  	} else {
>  		*p_after = 1;
>  	}