Re: [PATCH v4 2/2] patch-id: replace `atoi()` with `strtol_i_updated()`
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 24, 2024, 21:02 UTC
- Message-ID
- <xmqqttn2bg2o.fsf@gitster.g>
- In-Reply-To
- <17f2dda4907ec03b0783160c53c4896fd76cb053.1706079304.git.gitgitgadget@gmail.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.
Show 13 quoted lines
> } 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;
> }