On Thursday, January 25th, 2024 at 2:32 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > "Mohit Marathe via GitGitGadget" gitgitgadget@xxxxxxxxx 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. > I was confused about how an invalid hunk header of a corrupted would look like. This was just an attempt of making a sanity check. But after taking another look, I agree that its unnecessary. > > } 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; > > }