{"thread":{"id":"60779","subject":"[PATCH 0/2] Replace atoi() with strtol_i2()","startedAt":"2024-01-22T08:52:02Z","lastAt":"2024-02-04T05:48:44Z","messageCount":24,"participants":["Mohit Marathe via GitGitGadget","Junio C Hamano","Mohit Marathe"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"487188","messageId":"pull.1646.git.1705913519.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":null,"subject":"[PATCH 0/2] Replace atoi() with strtol_i2()","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-22T08:51:57Z","receivedAt":"2024-01-22T08:52:02Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"Hello,\n\nThis patch series replaces atoi() with an updated version of strtol_i()\ncalled strtol_i2 (Credits: Junio C Hamano). The reasoning behind this is to\nimprove error handling by not allowing non-numerical characters in the hunk\nheader (which might happen in case of a corrupt patch, although rarely).\n\nThere is still a change to be made, as Junio says: \"A corrupt patch may be\ngetting a nonsense patch-ID with the current code and hopefully is not\nmatching other patches that are not corrupt, but with such a change, a\ncorrupt patch may not be getting any patch-ID and a loop that computes\npatch-ID for many files and try to match them up might need to be rewritten\nto take the new failure case into account.\" I'm not sure where this change\nneeds to me made (maybe get_one_patchid()?). It would be great if anyone\ncould point me to the correct place.\n\nThanks, Mohit Marathe\n\nMohit Marathe (2):\n git-compat-util: add strtol_i2\n patch-id: replace `atoi()` with `strtol_i2()`\n\n builtin/patch-id.c | 14 +++++++-------\n git-compat-util.h | 23 +++++++++++++++++++++++\n 2 files changed, 30 insertions(+), 7 deletions(-)\n\n\nbase-commit: e02ecfcc534e2021aae29077a958dd11c3897e4c\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1646%2Fmohit-marathe%2Fupdate-strtol_i-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1646/mohit-marathe/update-strtol_i-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1646\n-- \ngitgitgadget\n"},{"id":"487189","messageId":"4e2b03cdd4f57d91f1993c5e89e28f9115033161.1705913519.git.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.git.1705913519.gitgitgadget@gmail.com","subject":"[PATCH 1/2] git-compat-util: add strtol_i2","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-22T08:51:58Z","receivedAt":"2024-01-22T08:52:03Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"From: Mohit Marathe \n\nThis function is an updated version of strtol_i function. It will\ngive more control to handle parsing of the characters after the\ninteger and better error handling while parsing numbers.\n\nSigned-off-by: Mohit Marathe \n---\n git-compat-util.h | 23 +++++++++++++++++++++++\n 1 file changed, 23 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 7c2a6538e5a..0a014c837d8 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -1309,6 +1309,29 @@ static inline int strtol_i(char const *s, int base, int *result)\n \treturn 0;\n }\n \n+#define strtol_i(s,b,r) strtol_i2((s), (b), (r), NULL)\n+static inline int strtol_i2(char const *s, int base, int *result, char **endp)\n+{\n+\tlong ul;\n+\tchar *dummy = NULL;\n+\n+\tif (!endp)\n+\t\tendp = &dummy;\n+\terrno = 0;\n+\tul = strtol(s, endp, base);\n+\tif (errno ||\n+\t /*\n+\t * if we are told to parse to the end of the string by\n+\t * passing NULL to endp, it is an error to have any\n+\t * remaining character after the digits.\n+\t */\n+\t (dummy && *dummy) ||\n+\t *endp == s || (int) ul != ul)\n+\t\treturn -1;\n+\t*result = ul;\n+\treturn 0;\n+}\n+\n void git_stable_qsort(void *base, size_t nmemb, size_t size,\n \t\t int(*compar)(const void *, const void *));\n #ifdef INTERNAL_QSORT\n-- \ngitgitgadget\n\n"},{"id":"487190","messageId":"1ece724b1ca7f71542bfef42795fce798563ecde.1705913519.git.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.git.1705913519.gitgitgadget@gmail.com","subject":"[PATCH 2/2] patch-id: replace `atoi()` with `strtol_i2()`","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-22T08:51:59Z","receivedAt":"2024-01-22T08:52:05Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"From: Mohit Marathe \n\nThe change is made to improve the error-handling capabilities\nduring the conversion of string representations to integers.\nThe `strtol_i2(` function offers a more robust mechanism for\nconverting strings to integers by providing enhanced error\ndetection. Unlike `atoi(`, `strtol_i2(` allows the code to\ndifferentiate between a valid conversion and an invalid one,\noffering better resilience against potential issues such as\nreading hunk header of a corrupted patch.\n\nSigned-off-by: Mohit Marathe \n---\n builtin/patch-id.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 3894d2b9706..6bfb263de5e 100644\n--- a/builtin/patch-id.c\n+++ b/builtin/patch-id.c\n@@ -1,3 +1,4 @@\n+#include \"git-compat-util.h\"\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"diff.h\"\n@@ -29,33 +30,32 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n {\n \tstatic const char digits[] = \"0123456789\";\n \tconst char *q, *r;\n+\tchar *endp;\n \tint n;\n \n \tq = p + 4;\n \tn = strspn(q, digits);\n \tif (q[n] == ',') {\n \t\tq += n + 1;\n-\t\t*p_before = atoi(q);\n+\t\tif (strtol_i2(q, 10, p_before, &endp) != 0)\n+\t\t\treturn 0;\n \t\tn = strspn(q, digits);\n \t} else {\n \t\t*p_before = 1;\n \t}\n \n-\tif (n == 0 || q[n] != ' ' || q[n+1] != '+')\n+\tif (q[n] != ' ' || q[n+1] != '+')\n \t\treturn 0;\n \n \tr = q + n + 2;\n \tn = strspn(r, digits);\n \tif (r[n] == ',') {\n \t\tr += n + 1;\n-\t\t*p_after = atoi(r);\n-\t\tn = strspn(r, digits);\n+\t\tif (strtol_i2(r, 10, p_after, &endp) != 0)\n+\t\t\treturn 0;\n \t} else {\n \t\t*p_after = 1;\n \t}\n-\tif (n == 0)\n-\t\treturn 0;\n-\n \treturn 1;\n }\n \n-- \ngitgitgadget\n"},{"id":"487214","messageId":"xmqq8r4htb8r.fsf@gitster.g","threadId":"60779","inReplyTo":"1ece724b1ca7f71542bfef42795fce798563ecde.1705913519.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] patch-id: replace `atoi()` with `strtol_i2()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-22T19:32:36Z","receivedAt":"2024-01-22T19:32:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Mohit Marathe via GitGitGadget\" writes:\n\n> \tstatic const char digits[] = \"0123456789\";\n> \tconst char *q, *r;\n> +\tchar *endp;\n> \tint n;\n> \n> \tq = p + 4;\n> \tn = strspn(q, digits);\n> \tif (q[n] == ',') {\n> \t\tq += n + 1;\n> -\t\t*p_before = atoi(q);\n> +\t\tif (strtol_i2(q, 10, p_before, &endp) != 0)\n> +\t\t\treturn 0;\n> \t\tn = strspn(q, digits);\n> \t} else {\n> \t\t*p_before = 1;\n> \t}\n\nLooking at this code again, because we upfront run strspn() to make\nsure q[] begins with a run of digits *and* followed by a comma\n(which is not a digit), I think it is safe to use atoi() and assume\nit would slurp all the digits. So the lack of another check the use\nof new helper allows us to do, namely\n\n\t\tif (endp != q + n)\n\t\t\treturn 0;\n\nis probably OK, but that is one of the two reasons why you would\nfavor the use of new helper over atoi(), so the upside of this\nchange is not all that great as I originally hoped for X-<.\n\nNot your fault, of course. We would still catch when the digit\nstring that starts q[] is too large to fit in an int, which is an\nupside.\n\n> -\tif (n == 0 || q[n] != ' ' || q[n+1] != '+')\n> +\tif (q[n] != ' ' || q[n+1] != '+')\n> \t\treturn 0;\n\nWhen we saw q[] that begins with ',' upon entry to this function, we\nused to say *p_before = 1 and then saw n==0 and realized it is not a\ngood input and returned 0 from the function.\n\nNow we instead peek q[0] and the check says q[0] is not SP so we\nwill return 0 the same way so there is no behaviour change from the\nupper hunk? The conversion may be correct, but it wasn't explained\nin the proposed commit log message.\n\nHow are the change to stop caring about n==0 here ...\n\n> \tr = q + n + 2;\n> \tn = strspn(r, digits);\n> \tif (r[n] == ',') {\n> \t\tr += n + 1;\n> -\t\t*p_after = atoi(r);\n> -\t\tn = strspn(r, digits);\n> +\t\tif (strtol_i2(r, 10, p_after, &endp) != 0)\n> +\t\t\treturn 0;\n> \t} else {\n> \t\t*p_after = 1;\n> \t}\n> -\tif (n == 0)\n> -\t\treturn 0;\n\n... and this change here, linked to the switch from atoi() to\nstrtul_i2()[*]?\n\nIt looks like an unrelated behaviour change that is left\nunexplained.\n\n> \treturn 1;\n> }\n\nThanks for working on this one.\n\n\n[Footnote]\n\n * by the way, what a horrible name for a public function. Yuck.\n"},{"id":"487295","messageId":"cwwH6NrWMc0fRUiirzvQmY-FoUS12K1SLSkBAbbu_QgAR7aDlvby6hgI_puXwtRSwX86zw6FQjUs1yHcPm8LcxuWhDnfG-4f0XRMjtTIVwU=@proton.me","threadId":"60779","inReplyTo":"xmqq8r4htb8r.fsf@gitster.g","subject":"Re: [PATCH 2/2] patch-id: replace `atoi()` with `strtol_i2()`","fromName":"Mohit Marathe","fromEmail":"mohitmarathe@proton.me","sentAt":"2024-01-24T04:22:04Z","receivedAt":"2024-01-24T04:22:24Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"\nOn Tuesday, January 23rd, 2024 at 1:02 AM, Junio C Hamano wrote:\n\n> \"Mohit Marathe via GitGitGadget\" gitgitgadget@gmail.com writes:\n> \n> > static const char digits[] = \"0123456789\";\n> > const char *q, *r;\n> > + char *endp;\n> > int n;\n> > \n> > q = p + 4;\n> > n = strspn(q, digits);\n> > if (q[n] == ',') {\n> > q += n + 1;\n> > - *p_before = atoi(q);\n> > + if (strtol_i2(q, 10, p_before, &endp) != 0)\n> > + return 0;\n> > n = strspn(q, digits);\n> > } else {\n> > *p_before = 1;\n> > }\n> \n> \n> Looking at this code again, because we upfront run strspn() to make\n> sure q[] begins with a run of digits and followed by a comma\n> (which is not a digit), I think it is safe to use atoi() and assume\n> it would slurp all the digits. So the lack of another check the use\n> of new helper allows us to do, namely\n> \n> if (endp != q + n)\n> return 0;\n> \n> is probably OK, but that is one of the two reasons why you would\n> favor the use of new helper over atoi(), so the upside of this\n> change is not all that great as I originally hoped for X-<.\n> \n> Not your fault, of course. We would still catch when the digit\n> string that starts q[] is too large to fit in an int, which is an\n> upside.\n> \n> > - if (n == 0 || q[n] != ' ' || q[n+1] != '+')\n> > + if (q[n] != ' ' || q[n+1] != '+')\n> > return 0;\n> \n> \n> When we saw q[] that begins with ',' upon entry to this function, we\n> used to say *p_before = 1 and then saw n==0 and realized it is not a\n> good input and returned 0 from the function.\n\nUh oh, I just looked at the `if` block and concluded that it was just \nto check if it has numbers after the ',', which`strtol_i2()` already \ndoes. But I totally missed this one. \n\n> Now we instead peek q[0] and the check says q[0] is not SP so we\n> will return 0 the same way so there is no behaviour change from the\n> upper hunk? The conversion may be correct, but it wasn't explained\n> in the proposed commit log message.\n> \n> How are the change to stop caring about n==0 here ...\n> \n> > r = q + n + 2;\n> > n = strspn(r, digits);\n> > if (r[n] == ',') {\n> > r += n + 1;\n> > - *p_after = atoi(r);\n> > - n = strspn(r, digits);\n> > + if (strtol_i2(r, 10, p_after, &endp) != 0)\n> > + return 0;\n> > } else {\n> > *p_after = 1;\n> > }\n> > - if (n == 0)\n> > - return 0;\n> \n> \n> ... and this change here, linked to the switch from atoi() to\n> strtul_i2()[*]?\n> \n> It looks like an unrelated behaviour change that is left\n> unexplained.\n> \n> > return 1;\n> > }\n> \n> \n> Thanks for working on this one.\n> \n> \n> [Footnote]\n> \n> * by the way, what a horrible name for a public function. Yuck.\n\nYeah, I thought so too /:D How does `strtol_i_updated` sounds?\n\nThanks for you feedback! I will send v2 with the corrections soon.\n"},{"id":"487297","messageId":"60ea85a701a05831b0adf1e3f9a7a97fd31ef43f.1706077977.git.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v2.git.1706077977.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] git-compat-util: add strtol_i_updated()","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-24T06:32:56Z","receivedAt":"2024-01-24T06:33:01Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"From: Mohit Marathe \n\nThis function is an updated version of strtol_i() function. It will\ngive more control to handle parsing of the characters after the\ninteger and better error handling while parsing numbers.\n\nSigned-off-by: Mohit Marathe \n---\n git-compat-util.h | 23 +++++++++++++++++++++++\n 1 file changed, 23 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 7c2a6538e5a..b38d7c7f8f1 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -1309,6 +1309,29 @@ static inline int strtol_i(char const *s, int base, int *result)\n \treturn 0;\n }\n \n+#define strtol_i(s,b,r) strtol_i_updated((s), (b), (r), NULL)\n+static inline int strtol_i_updated(char const *s, int base, int *result, char **endp)\n+{\n+\tlong ul;\n+\tchar *dummy = NULL;\n+\n+\tif (!endp)\n+\t\tendp = &dummy;\n+\terrno = 0;\n+\tul = strtol(s, endp, base);\n+\tif (errno ||\n+\t /*\n+\t * if we are told to parse to the end of the string by\n+\t * passing NULL to endp, it is an error to have any\n+\t * remaining character after the digits.\n+\t */\n+\t (dummy && *dummy) ||\n+\t *endp == s || (int) ul != ul)\n+\t\treturn -1;\n+\t*result = ul;\n+\treturn 0;\n+}\n+\n void git_stable_qsort(void *base, size_t nmemb, size_t size,\n \t\t int(*compar)(const void *, const void *));\n #ifdef INTERNAL_QSORT\n-- \ngitgitgadget\n\n"},{"id":"487298","messageId":"pull.1646.v2.git.1706077977.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.git.1705913519.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] Replace atoi() with strtol_i_updated()","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-24T06:32:55Z","receivedAt":"2024-01-24T06:33:01Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"Hello,\n\nThis patch series replaces atoi() with an updated version of strtol_i()\ncalled strtol_i_updated (Credits: Junio C Hamano). The reasoning behind this\nis to improve error handling by not allowing non-numerical characters in the\nhunk header (which might happen in case of a corrupt patch, although\nrarely).\n\nThere is still a change to be made, as Junio says: \"A corrupt patch may be\ngetting a nonsense patch-ID with the current code and hopefully is not\nmatching other patches that are not corrupt, but with such a change, a\ncorrupt patch may not be getting any patch-ID and a loop that computes\npatch-ID for many files and try to match them up might need to be rewritten\nto take the new failure case into account.\" I'm not sure where this change\nneeds to me made (maybe get_one_patchid()?). It would be great if anyone\ncould point me to the correct place.\n\nThanks, Mohit Marathe\n\nMohit Marathe (2):\n git-compat-util: add strtol_i_updated()\n patch-id: replace `atoi()` with `strtol_i_updated()`\n\n builtin/patch-id.c | 8 ++++++--\n git-compat-util.h | 23 +++++++++++++++++++++++\n 2 files changed, 29 insertions(+), 2 deletions(-)\n\n\nbase-commit: e02ecfcc534e2021aae29077a958dd11c3897e4c\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1646%2Fmohit-marathe%2Fupdate-strtol_i-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1646/mohit-marathe/update-strtol_i-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1646\n\nRange-diff vs v1:\n\n 1: 4e2b03cdd4f ! 1: 60ea85a701a git-compat-util: add strtol_i2\n @@ Metadata\n Author: Mohit Marathe \n \n ## Commit message ##\n - git-compat-util: add strtol_i2\n + git-compat-util: add strtol_i_updated()\n \n - This function is an updated version of strtol_i function. It will\n + This function is an updated version of strtol_i() function. It will\n give more control to handle parsing of the characters after the\n integer and better error handling while parsing numbers.\n \n @@ git-compat-util.h: static inline int strtol_i(char const *s, int base, int *resu\n \treturn 0;\n }\n \n -+#define strtol_i(s,b,r) strtol_i2((s), (b), (r), NULL)\n -+static inline int strtol_i2(char const *s, int base, int *result, char **endp)\n ++#define strtol_i(s,b,r) strtol_i_updated((s), (b), (r), NULL)\n ++static inline int strtol_i_updated(char const *s, int base, int *result, char **endp)\n +{\n +\tlong ul;\n +\tchar *dummy = NULL;\n 2: 1ece724b1ca ! 2: f3a03d68211 patch-id: replace `atoi()` with `strtol_i2()`\n @@ Metadata\n Author: Mohit Marathe \n \n ## Commit message ##\n - patch-id: replace `atoi()` with `strtol_i2()`\n + patch-id: replace `atoi()` with `strtol_i_updated()`\n \n The change is made to improve the error-handling capabilities\n during the conversion of string representations to integers.\n - The `strtol_i2(` function offers a more robust mechanism for\n + The `strtol_i_updated(` function offers a more robust mechanism for\n converting strings to integers by providing enhanced error\n - detection. Unlike `atoi(`, `strtol_i2(` allows the code to\n + detection. Unlike `atoi(`, `strtol_i_updated(` allows the code to\n differentiate between a valid conversion and an invalid one,\n offering better resilience against potential issues such as\n reading hunk header of a corrupted patch.\n @@ builtin/patch-id.c: static int scan_hunk_header(const char *p, int *p_before, in\n \tif (q[n] == ',') {\n \t\tq += n + 1;\n -\t\t*p_before = atoi(q);\n -+\t\tif (strtol_i2(q, 10, p_before, &endp) != 0)\n ++\t\tif (strtol_i_updated(q, 10, p_before, &endp) != 0)\n +\t\t\treturn 0;\n \t\tn = strspn(q, digits);\n \t} else {\n \t\t*p_before = 1;\n - \t}\n - \n --\tif (n == 0 || q[n] != ' ' || q[n+1] != '+')\n -+\tif (q[n] != ' ' || q[n+1] != '+')\n - \t\treturn 0;\n - \n - \tr = q + n + 2;\n +@@ builtin/patch-id.c: static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n \tn = strspn(r, digits);\n \tif (r[n] == ',') {\n \t\tr += n + 1;\n -\t\t*p_after = atoi(r);\n --\t\tn = strspn(r, digits);\n -+\t\tif (strtol_i2(r, 10, p_after, &endp) != 0)\n ++\t\tif (strtol_i_updated(r, 10, p_after, &endp) != 0)\n +\t\t\treturn 0;\n + \t\tn = strspn(r, digits);\n \t} else {\n \t\t*p_after = 1;\n - \t}\n --\tif (n == 0)\n --\t\treturn 0;\n --\n - \treturn 1;\n - }\n - \n\n-- \ngitgitgadget\n"},{"id":"487299","messageId":"f3a03d68211bc79b41562762c8ca63889b9218b2.1706077977.git.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v2.git.1706077977.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] patch-id: replace `atoi()` with `strtol_i_updated()`","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-24T06:32:57Z","receivedAt":"2024-01-24T06:33:04Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"From: Mohit Marathe \n\nThe change is made to improve the error-handling capabilities\nduring the conversion of string representations to integers.\nThe `strtol_i_updated(` function offers a more robust mechanism for\nconverting strings to integers by providing enhanced error\ndetection. Unlike `atoi(`, `strtol_i_updated(` allows the code to\ndifferentiate between a valid conversion and an invalid one,\noffering better resilience against potential issues such as\nreading hunk header of a corrupted patch.\n\nSigned-off-by: Mohit Marathe \n---\n builtin/patch-id.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 3894d2b9706..e513b6aed3f 100644\n--- a/builtin/patch-id.c\n+++ b/builtin/patch-id.c\n@@ -1,3 +1,4 @@\n+#include \"git-compat-util.h\"\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"diff.h\"\n@@ -29,13 +30,15 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n {\n \tstatic const char digits[] = \"0123456789\";\n \tconst char *q, *r;\n+\tchar *endp;\n \tint n;\n \n \tq = p + 4;\n \tn = strspn(q, digits);\n \tif (q[n] == ',') {\n \t\tq += n + 1;\n-\t\t*p_before = atoi(q);\n+\t\tif (strtol_i_updated(q, 10, p_before, &endp) != 0)\n+\t\t\treturn 0;\n \t\tn = strspn(q, digits);\n \t} else {\n \t\t*p_before = 1;\n@@ -48,7 +51,8 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n \tn = strspn(r, digits);\n \tif (r[n] == ',') {\n \t\tr += n + 1;\n-\t\t*p_after = atoi(r);\n+\t\tif (strtol_i_updated(r, 10, p_after, &endp) != 0)\n+\t\t\treturn 0;\n \t\tn = strspn(r, digits);\n \t} else {\n \t\t*p_after = 1;\n-- \ngitgitgadget\n"},{"id":"487300","messageId":"pull.1646.v3.git.1706078885.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v2.git.1706077977.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] Replace atoi() with strtol_i_updated()","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-24T06:48:03Z","receivedAt":"2024-01-24T06:48:08Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"Hello,\n\nThis patch series replaces atoi() with an updated version of strtol_i()\ncalled strtol_i_updated (Credits: Junio C Hamano). The reasoning behind this\nis to improve error handling by not allowing non-numerical characters in the\nhunk header (which might happen in case of a corrupt patch, although\nrarely).\n\nThere is still a change to be made, as Junio says: \"A corrupt patch may be\ngetting a nonsense patch-ID with the current code and hopefully is not\nmatching other patches that are not corrupt, but with such a change, a\ncorrupt patch may not be getting any patch-ID and a loop that computes\npatch-ID for many files and try to match them up might need to be rewritten\nto take the new failure case into account.\" I'm not sure where this change\nneeds to me made (maybe get_one_patchid()?). It would be great if anyone\ncould point me to the correct place.\n\nThanks, Mohit Marathe\n\nMohit Marathe (2):\n git-compat-util: add strtol_i_updated()\n patch-id: replace `atoi()` with `strtol_i_updated()`\n\n builtin/patch-id.c | 12 ++++++++++--\n git-compat-util.h | 23 +++++++++++++++++++++++\n 2 files changed, 33 insertions(+), 2 deletions(-)\n\n\nbase-commit: e02ecfcc534e2021aae29077a958dd11c3897e4c\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1646%2Fmohit-marathe%2Fupdate-strtol_i-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1646/mohit-marathe/update-strtol_i-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1646\n\nRange-diff vs v2:\n\n 1: 60ea85a701a = 1: 60ea85a701a git-compat-util: add strtol_i_updated()\n 2: f3a03d68211 ! 2: 0e117198d01 patch-id: replace `atoi()` with `strtol_i_updated()`\n @@ builtin/patch-id.c: static int scan_hunk_header(const char *p, int *p_before, in\n +\t\tif (strtol_i_updated(q, 10, p_before, &endp) != 0)\n +\t\t\treturn 0;\n \t\tn = strspn(q, digits);\n ++\t\tif (endp != q + n)\n ++\t\t\treturn 0;\n \t} else {\n \t\t*p_before = 1;\n + \t}\n @@ builtin/patch-id.c: static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n \tn = strspn(r, digits);\n \tif (r[n] == ',') {\n @@ builtin/patch-id.c: static int scan_hunk_header(const char *p, int *p_before, in\n +\t\tif (strtol_i_updated(r, 10, p_after, &endp) != 0)\n +\t\t\treturn 0;\n \t\tn = strspn(r, digits);\n ++\t\tif (endp != q + n)\n ++\t\t\treturn 0;\n \t} else {\n \t\t*p_after = 1;\n + \t}\n\n-- \ngitgitgadget\n"},{"id":"487301","messageId":"60ea85a701a05831b0adf1e3f9a7a97fd31ef43f.1706078885.git.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v3.git.1706078885.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] git-compat-util: add strtol_i_updated()","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-24T06:48:04Z","receivedAt":"2024-01-24T06:48:08Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"From: Mohit Marathe \n\nThis function is an updated version of strtol_i() function. It will\ngive more control to handle parsing of the characters after the\ninteger and better error handling while parsing numbers.\n\nSigned-off-by: Mohit Marathe \n---\n git-compat-util.h | 23 +++++++++++++++++++++++\n 1 file changed, 23 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 7c2a6538e5a..b38d7c7f8f1 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -1309,6 +1309,29 @@ static inline int strtol_i(char const *s, int base, int *result)\n \treturn 0;\n }\n \n+#define strtol_i(s,b,r) strtol_i_updated((s), (b), (r), NULL)\n+static inline int strtol_i_updated(char const *s, int base, int *result, char **endp)\n+{\n+\tlong ul;\n+\tchar *dummy = NULL;\n+\n+\tif (!endp)\n+\t\tendp = &dummy;\n+\terrno = 0;\n+\tul = strtol(s, endp, base);\n+\tif (errno ||\n+\t /*\n+\t * if we are told to parse to the end of the string by\n+\t * passing NULL to endp, it is an error to have any\n+\t * remaining character after the digits.\n+\t */\n+\t (dummy && *dummy) ||\n+\t *endp == s || (int) ul != ul)\n+\t\treturn -1;\n+\t*result = ul;\n+\treturn 0;\n+}\n+\n void git_stable_qsort(void *base, size_t nmemb, size_t size,\n \t\t int(*compar)(const void *, const void *));\n #ifdef INTERNAL_QSORT\n-- \ngitgitgadget\n\n"},{"id":"487302","messageId":"0e117198d0144fa4ee6ccf01ec31a099eaf8121c.1706078885.git.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v3.git.1706078885.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] patch-id: replace `atoi()` with `strtol_i_updated()`","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-24T06:48:05Z","receivedAt":"2024-01-24T06:48:09Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"From: Mohit Marathe \n\nThe change is made to improve the error-handling capabilities\nduring the conversion of string representations to integers.\nThe `strtol_i_updated(` function offers a more robust mechanism for\nconverting strings to integers by providing enhanced error\ndetection. Unlike `atoi(`, `strtol_i_updated(` allows the code to\ndifferentiate between a valid conversion and an invalid one,\noffering better resilience against potential issues such as\nreading hunk header of a corrupted patch.\n\nSigned-off-by: Mohit Marathe \n---\n builtin/patch-id.c | 12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 3894d2b9706..2c00d45cb2c 100644\n--- a/builtin/patch-id.c\n+++ b/builtin/patch-id.c\n@@ -1,3 +1,4 @@\n+#include \"git-compat-util.h\"\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"diff.h\"\n@@ -29,14 +30,18 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n {\n \tstatic const char digits[] = \"0123456789\";\n \tconst char *q, *r;\n+\tchar *endp;\n \tint n;\n \n \tq = p + 4;\n \tn = strspn(q, digits);\n \tif (q[n] == ',') {\n \t\tq += n + 1;\n-\t\t*p_before = atoi(q);\n+\t\tif (strtol_i_updated(q, 10, p_before, &endp) != 0)\n+\t\t\treturn 0;\n \t\tn = strspn(q, digits);\n+\t\tif (endp != q + n)\n+\t\t\treturn 0;\n \t} else {\n \t\t*p_before = 1;\n \t}\n@@ -48,8 +53,11 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n \tn = strspn(r, digits);\n \tif (r[n] == ',') {\n \t\tr += n + 1;\n-\t\t*p_after = atoi(r);\n+\t\tif (strtol_i_updated(r, 10, p_after, &endp) != 0)\n+\t\t\treturn 0;\n \t\tn = strspn(r, digits);\n+\t\tif (endp != q + n)\n+\t\t\treturn 0;\n \t} else {\n \t\t*p_after = 1;\n \t}\n-- \ngitgitgadget\n"},{"id":"487304","messageId":"pull.1646.v4.git.1706079304.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v3.git.1706078885.gitgitgadget@gmail.com","subject":"[PATCH v4 0/2] Replace atoi() with strtol_i_updated()","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-24T06:55:02Z","receivedAt":"2024-01-24T06:55:07Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"Hello,\n\nThis patch series replaces atoi() with an updated version of strtol_i()\ncalled strtol_i_updated (Credits: Junio C Hamano). The reasoning behind this\nis to improve error handling by not allowing non-numerical characters in the\nhunk header (which might happen in case of a corrupt patch, although\nrarely).\n\nThere is still a change to be made, as Junio says: \"A corrupt patch may be\ngetting a nonsense patch-ID with the current code and hopefully is not\nmatching other patches that are not corrupt, but with such a change, a\ncorrupt patch may not be getting any patch-ID and a loop that computes\npatch-ID for many files and try to match them up might need to be rewritten\nto take the new failure case into account.\" I'm not sure where this change\nneeds to me made (maybe get_one_patchid()?). It would be great if anyone\ncould point me to the correct place.\n\nThanks, Mohit Marathe\n\nMohit Marathe (2):\n git-compat-util: add strtol_i_updated()\n patch-id: replace `atoi()` with `strtol_i_updated()`\n\n builtin/patch-id.c | 12 ++++++++++--\n git-compat-util.h | 23 +++++++++++++++++++++++\n 2 files changed, 33 insertions(+), 2 deletions(-)\n\n\nbase-commit: e02ecfcc534e2021aae29077a958dd11c3897e4c\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1646%2Fmohit-marathe%2Fupdate-strtol_i-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1646/mohit-marathe/update-strtol_i-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/1646\n\nRange-diff vs v3:\n\n 1: 60ea85a701a = 1: 60ea85a701a git-compat-util: add strtol_i_updated()\n 2: 0e117198d01 ! 2: 17f2dda4907 patch-id: replace `atoi()` with `strtol_i_updated()`\n @@ builtin/patch-id.c: static int scan_hunk_header(const char *p, int *p_before, in\n +\t\tif (strtol_i_updated(r, 10, p_after, &endp) != 0)\n +\t\t\treturn 0;\n \t\tn = strspn(r, digits);\n -+\t\tif (endp != q + n)\n ++\t\tif (endp != r + n)\n +\t\t\treturn 0;\n \t} else {\n \t\t*p_after = 1;\n\n-- \ngitgitgadget\n"},{"id":"487305","messageId":"60ea85a701a05831b0adf1e3f9a7a97fd31ef43f.1706079304.git.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v4.git.1706079304.gitgitgadget@gmail.com","subject":"[PATCH v4 1/2] git-compat-util: add strtol_i_updated()","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-24T06:55:03Z","receivedAt":"2024-01-24T06:55:08Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"From: Mohit Marathe \n\nThis function is an updated version of strtol_i() function. It will\ngive more control to handle parsing of the characters after the\ninteger and better error handling while parsing numbers.\n\nSigned-off-by: Mohit Marathe \n---\n git-compat-util.h | 23 +++++++++++++++++++++++\n 1 file changed, 23 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 7c2a6538e5a..b38d7c7f8f1 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -1309,6 +1309,29 @@ static inline int strtol_i(char const *s, int base, int *result)\n \treturn 0;\n }\n \n+#define strtol_i(s,b,r) strtol_i_updated((s), (b), (r), NULL)\n+static inline int strtol_i_updated(char const *s, int base, int *result, char **endp)\n+{\n+\tlong ul;\n+\tchar *dummy = NULL;\n+\n+\tif (!endp)\n+\t\tendp = &dummy;\n+\terrno = 0;\n+\tul = strtol(s, endp, base);\n+\tif (errno ||\n+\t /*\n+\t * if we are told to parse to the end of the string by\n+\t * passing NULL to endp, it is an error to have any\n+\t * remaining character after the digits.\n+\t */\n+\t (dummy && *dummy) ||\n+\t *endp == s || (int) ul != ul)\n+\t\treturn -1;\n+\t*result = ul;\n+\treturn 0;\n+}\n+\n void git_stable_qsort(void *base, size_t nmemb, size_t size,\n \t\t int(*compar)(const void *, const void *));\n #ifdef INTERNAL_QSORT\n-- \ngitgitgadget\n\n"},{"id":"487306","messageId":"17f2dda4907ec03b0783160c53c4896fd76cb053.1706079304.git.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v4.git.1706079304.gitgitgadget@gmail.com","subject":"[PATCH v4 2/2] patch-id: replace `atoi()` with `strtol_i_updated()`","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-24T06:55:04Z","receivedAt":"2024-01-24T06:55:09Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"From: Mohit Marathe \n\nThe change is made to improve the error-handling capabilities\nduring the conversion of string representations to integers.\nThe `strtol_i_updated(` function offers a more robust mechanism for\nconverting strings to integers by providing enhanced error\ndetection. Unlike `atoi(`, `strtol_i_updated(` allows the code to\ndifferentiate between a valid conversion and an invalid one,\noffering better resilience against potential issues such as\nreading hunk header of a corrupted patch.\n\nSigned-off-by: Mohit Marathe \n---\n builtin/patch-id.c | 12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 3894d2b9706..88db178c905 100644\n--- a/builtin/patch-id.c\n+++ b/builtin/patch-id.c\n@@ -1,3 +1,4 @@\n+#include \"git-compat-util.h\"\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"diff.h\"\n@@ -29,14 +30,18 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n {\n \tstatic const char digits[] = \"0123456789\";\n \tconst char *q, *r;\n+\tchar *endp;\n \tint n;\n \n \tq = p + 4;\n \tn = strspn(q, digits);\n \tif (q[n] == ',') {\n \t\tq += n + 1;\n-\t\t*p_before = atoi(q);\n+\t\tif (strtol_i_updated(q, 10, p_before, &endp) != 0)\n+\t\t\treturn 0;\n \t\tn = strspn(q, digits);\n+\t\tif (endp != q + n)\n+\t\t\treturn 0;\n \t} else {\n \t\t*p_before = 1;\n \t}\n@@ -48,8 +53,11 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n \tn = strspn(r, digits);\n \tif (r[n] == ',') {\n \t\tr += n + 1;\n-\t\t*p_after = atoi(r);\n+\t\tif (strtol_i_updated(r, 10, p_after, &endp) != 0)\n+\t\t\treturn 0;\n \t\tn = strspn(r, digits);\n+\t\tif (endp != r + n)\n+\t\t\treturn 0;\n \t} else {\n \t\t*p_after = 1;\n \t}\n-- \ngitgitgadget\n"},{"id":"487351","messageId":"xmqqede6cwkg.fsf@gitster.g","threadId":"60779","inReplyTo":"60ea85a701a05831b0adf1e3f9a7a97fd31ef43f.1706079304.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 1/2] git-compat-util: add strtol_i_updated()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-24T20:20:47Z","receivedAt":"2024-01-24T20:20:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Mohit Marathe via GitGitGadget\" writes:\n\n> From: Mohit Marathe \n>\n> This function is an updated version of strtol_i() function. It will\n> give more control to handle parsing of the characters after the\n> integer and better error handling while parsing numbers.\n\ni2 was horrible but this is worse. What would you call an even\nnewer variant when you need to add one? strtol_i_updated_twice?\n\nTo readers who are reading the code in 6 months, it is totally\nuninteresting that strtol_i() is an older function and the new thing\nwas invented later as its update. What they want to learn is how\nthese two are different, what additional things this new one lets\nthem do compared to the old one, namely: we can optionally learn\nwhere the run of the digits has ended.\n\nPerhaps call it \"strtoi_with_tail\" or something, unless others\nsuggest even better names?\n\nThanks.\n"},{"id":"487352","messageId":"xmqqttn2bg2o.fsf@gitster.g","threadId":"60779","inReplyTo":"17f2dda4907ec03b0783160c53c4896fd76cb053.1706079304.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 2/2] patch-id: replace `atoi()` with `strtol_i_updated()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-24T21:02:23Z","receivedAt":"2024-01-24T21:02:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Mohit Marathe via GitGitGadget\" writes:\n\n> \tq = p + 4;\n> \tn = strspn(q, digits);\n> \tif (q[n] == ',') {\n> \t\tq += n + 1;\n\nSo, we saw \"@@ -\" and skipped over these four bytes, skipped the\ndigits from there, and found a comma. \n\nFor \"@@ -29,14 +30,18 @@\", for example, our q is now \"14 +30,18 @@\"\nas we have skipped over that comma after 29.\n\n> -\t\t*p_before = atoi(q);\n> +\t\tif (strtol_i_updated(q, 10, p_before, &endp) != 0)\n> +\t\t\treturn 0;\n\nWe parse out 14 and store it to *p_before. endp points at \" +30...\"\nnow.\n\n> \t\tn = strspn(q, digits);\n> +\t\tif (endp != q + n)\n> +\t\t\treturn 0;\n\nIs this necessary? By asking strtol_i_updated() where the number ended,\nwe already know endp without skipping the digits in q with strspn().\nShouldn't these three lines become more like\n\n\t\tn = endp - q;\n\ninstead? \n\nAfter all, we are not trying to find a bug in strtol_i_updated(),\nwhich would be the only reason how this \"return 0\" would trigger.\n\n> \t} else {\n> \t\t*p_before = 1;\n> \t}\n> @@ -48,8 +53,11 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n> \tn = strspn(r, digits);\n> \tif (r[n] == ',') {\n> \t\tr += n + 1;\n> -\t\t*p_after = atoi(r);\n> +\t\tif (strtol_i_updated(r, 10, p_after, &endp) != 0)\n> +\t\t\treturn 0;\n> \t\tn = strspn(r, digits);\n> +\t\tif (endp != r + n)\n> +\t\t\treturn 0;\n\nLikewise.\n\n> \t} else {\n> \t\t*p_after = 1;\n> \t}\n\n"},{"id":"487460","messageId":"0d_yXUwMsJQsIySLnVJ5SlpFi2YdTgnvVN5cLCbwt_V4pVOUipzFEPlFY1xLicHfe4SGsWwF5D1q1idGBVyRXXr6tME-GR6hQG7usi5mypw=@proton.me","threadId":"60779","inReplyTo":"xmqqttn2bg2o.fsf@gitster.g","subject":"Re: [PATCH v4 2/2] patch-id: replace `atoi()` with `strtol_i_updated()`","fromName":"Mohit Marathe","fromEmail":"mohitmarathe@proton.me","sentAt":"2024-01-28T04:35:02Z","receivedAt":"2024-01-28T04:35:19Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"\nOn Thursday, January 25th, 2024 at 2:32 AM, Junio C Hamano wrote:\n\n> \"Mohit Marathe via GitGitGadget\" gitgitgadget@gmail.com writes:\n> \n> > q = p + 4;\n> > n = strspn(q, digits);\n> > if (q[n] == ',') {\n> > q += n + 1;\n> \n> \n> So, we saw \"@@ -\" and skipped over these four bytes, skipped the\n> digits from there, and found a comma.\n> \n> For \"@@ -29,14 +30,18 @@\", for example, our q is now \"14 +30,18 @@\"\n> as we have skipped over that comma after 29.\n> \n> > - *p_before = atoi(q);\n> > + if (strtol_i_updated(q, 10, p_before, &endp) != 0)\n> > + return 0;\n> \n> \n> We parse out 14 and store it to *p_before. endp points at \" +30...\"\n> now.\n> \n> > n = strspn(q, digits);\n> > + if (endp != q + n)\n> > + return 0;\n> \n> \n> Is this necessary? By asking strtol_i_updated() where the number ended,\n> we already know endp without skipping the digits in q with strspn().\n> Shouldn't these three lines become more like\n> \n> n = endp - q;\n> \n> instead?\n> \n> After all, we are not trying to find a bug in strtol_i_updated(),\n> which would be the only reason how this \"return 0\" would trigger.\n> \n\nI was confused about how an invalid hunk header of a corrupted would\nlook like. This was just an attempt of making a sanity check. But after\ntaking another look, I agree that its unnecessary.\n\n> > } else {\n> > *p_before = 1;\n> > }\n> > @@ -48,8 +53,11 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n> > n = strspn(r, digits);\n> > if (r[n] == ',') {\n> > r += n + 1;\n> > - *p_after = atoi(r);\n> > + if (strtol_i_updated(r, 10, p_after, &endp) != 0)\n> > + return 0;\n> > n = strspn(r, digits);\n> > + if (endp != r + n)\n> > + return 0;\n> \n> \n> Likewise.\n> \n> > } else {\n> > *p_after = 1;\n> > }\n"},{"id":"487461","messageId":"pull.1646.v5.git.1706416952.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v4.git.1706079304.gitgitgadget@gmail.com","subject":"[PATCH v5 0/2] Replace atoi() with strtoi_with_tail()","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-28T04:42:30Z","receivedAt":"2024-01-28T04:42:35Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"Hello,\n\nThis patch series replaces atoi() with an updated version of strtol_i()\ncalled strtoi_with_tail (Credits: Junio C Hamano). The reasoning behind this\nis to improve error handling by not allowing non-numerical characters in the\nhunk header (which might happen in case of a corrupt patch, although\nrarely).\n\nThere is still a change to be made, as Junio says: \"A corrupt patch may be\ngetting a nonsense patch-ID with the current code and hopefully is not\nmatching other patches that are not corrupt, but with such a change, a\ncorrupt patch may not be getting any patch-ID and a loop that computes\npatch-ID for many files and try to match them up might need to be rewritten\nto take the new failure case into account.\" I'm not sure where this change\nneeds to me made (maybe get_one_patchid()?). It would be great if anyone\ncould point me to the correct place.\n\nThanks, Mohit Marathe\n\nMohit Marathe (2):\n git-compat-util: add strtoi_with_tail()\n patch-id: replace `atoi()` with `strtoi_with_tail`\n\n builtin/patch-id.c | 12 ++++++++----\n git-compat-util.h | 23 +++++++++++++++++++++++\n 2 files changed, 31 insertions(+), 4 deletions(-)\n\n\nbase-commit: b50a608ba20348cb3dfc16a696816d51780e3f0f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1646%2Fmohit-marathe%2Fupdate-strtol_i-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1646/mohit-marathe/update-strtol_i-v5\nPull-Request: https://github.com/gitgitgadget/git/pull/1646\n\nRange-diff vs v4:\n\n 1: 60ea85a701a ! 1: f09b0838f04 git-compat-util: add strtol_i_updated()\n @@ Metadata\n Author: Mohit Marathe \n \n ## Commit message ##\n - git-compat-util: add strtol_i_updated()\n + git-compat-util: add strtoi_with_tail()\n \n This function is an updated version of strtol_i() function. It will\n give more control to handle parsing of the characters after the\n - integer and better error handling while parsing numbers.\n + numbers and better error handling while parsing numbers.\n \n Signed-off-by: Mohit Marathe \n \n @@ git-compat-util.h: static inline int strtol_i(char const *s, int base, int *resu\n \treturn 0;\n }\n \n -+#define strtol_i(s,b,r) strtol_i_updated((s), (b), (r), NULL)\n -+static inline int strtol_i_updated(char const *s, int base, int *result, char **endp)\n ++#define strtol_i(s,b,r) strtoi_with_tail((s), (b), (r), NULL)\n ++static inline int strtoi_with_tail(char const *s, int base, int *result, char **endp)\n +{\n +\tlong ul;\n +\tchar *dummy = NULL;\n 2: 17f2dda4907 ! 2: ee8f4ae991d patch-id: replace `atoi()` with `strtol_i_updated()`\n @@ Metadata\n Author: Mohit Marathe \n \n ## Commit message ##\n - patch-id: replace `atoi()` with `strtol_i_updated()`\n + patch-id: replace `atoi()` with `strtoi_with_tail`\n \n The change is made to improve the error-handling capabilities\n - during the conversion of string representations to integers.\n - The `strtol_i_updated(` function offers a more robust mechanism for\n + during the conversion of string to integers. The\n + `strtoi_with_tail` function offers a more robust mechanism for\n converting strings to integers by providing enhanced error\n - detection. Unlike `atoi(`, `strtol_i_updated(` allows the code to\n + detection. Unlike `atoi`, `strtoi_with_tail` allows the code to\n differentiate between a valid conversion and an invalid one,\n offering better resilience against potential issues such as\n reading hunk header of a corrupted patch.\n @@ builtin/patch-id.c: static int scan_hunk_header(const char *p, int *p_before, in\n \tif (q[n] == ',') {\n \t\tq += n + 1;\n -\t\t*p_before = atoi(q);\n -+\t\tif (strtol_i_updated(q, 10, p_before, &endp) != 0)\n -+\t\t\treturn 0;\n - \t\tn = strspn(q, digits);\n -+\t\tif (endp != q + n)\n +-\t\tn = strspn(q, digits);\n ++\t\tif (strtoi_with_tail(q, 10, p_before, &endp) != 0)\n +\t\t\treturn 0;\n ++\t\tn = endp - q;\n \t} else {\n \t\t*p_before = 1;\n \t}\n @@ builtin/patch-id.c: static int scan_hunk_header(const char *p, int *p_before, in\n \tif (r[n] == ',') {\n \t\tr += n + 1;\n -\t\t*p_after = atoi(r);\n -+\t\tif (strtol_i_updated(r, 10, p_after, &endp) != 0)\n -+\t\t\treturn 0;\n - \t\tn = strspn(r, digits);\n -+\t\tif (endp != r + n)\n +-\t\tn = strspn(r, digits);\n ++\t\tif (strtoi_with_tail(r, 10, p_after, &endp) != 0)\n +\t\t\treturn 0;\n ++\t\tn = endp - r;\n \t} else {\n \t\t*p_after = 1;\n \t}\n\n-- \ngitgitgadget\n"},{"id":"487462","messageId":"f09b0838f049de3e4b8cc6adc6cd4492bac7e967.1706416952.git.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v5.git.1706416952.gitgitgadget@gmail.com","subject":"[PATCH v5 1/2] git-compat-util: add strtoi_with_tail()","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-28T04:42:31Z","receivedAt":"2024-01-28T04:42:36Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"From: Mohit Marathe \n\nThis function is an updated version of strtol_i() function. It will\ngive more control to handle parsing of the characters after the\nnumbers and better error handling while parsing numbers.\n\nSigned-off-by: Mohit Marathe \n---\n git-compat-util.h | 23 +++++++++++++++++++++++\n 1 file changed, 23 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 7c2a6538e5a..c576b1b104f 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -1309,6 +1309,29 @@ static inline int strtol_i(char const *s, int base, int *result)\n \treturn 0;\n }\n \n+#define strtol_i(s,b,r) strtoi_with_tail((s), (b), (r), NULL)\n+static inline int strtoi_with_tail(char const *s, int base, int *result, char **endp)\n+{\n+\tlong ul;\n+\tchar *dummy = NULL;\n+\n+\tif (!endp)\n+\t\tendp = &dummy;\n+\terrno = 0;\n+\tul = strtol(s, endp, base);\n+\tif (errno ||\n+\t /*\n+\t * if we are told to parse to the end of the string by\n+\t * passing NULL to endp, it is an error to have any\n+\t * remaining character after the digits.\n+\t */\n+\t (dummy && *dummy) ||\n+\t *endp == s || (int) ul != ul)\n+\t\treturn -1;\n+\t*result = ul;\n+\treturn 0;\n+}\n+\n void git_stable_qsort(void *base, size_t nmemb, size_t size,\n \t\t int(*compar)(const void *, const void *));\n #ifdef INTERNAL_QSORT\n-- \ngitgitgadget\n\n"},{"id":"487463","messageId":"ee8f4ae991d71d399a9c07086f56a38f7531b6cf.1706416952.git.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v5.git.1706416952.gitgitgadget@gmail.com","subject":"[PATCH v5 2/2] patch-id: replace `atoi()` with `strtoi_with_tail`","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-28T04:42:32Z","receivedAt":"2024-01-28T04:42:37Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"From: Mohit Marathe \n\nThe change is made to improve the error-handling capabilities\nduring the conversion of string to integers. The\n`strtoi_with_tail` function offers a more robust mechanism for\nconverting strings to integers by providing enhanced error\ndetection. Unlike `atoi`, `strtoi_with_tail` allows the code to\ndifferentiate between a valid conversion and an invalid one,\noffering better resilience against potential issues such as\nreading hunk header of a corrupted patch.\n\nSigned-off-by: Mohit Marathe \n---\n builtin/patch-id.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 3894d2b9706..4e9a301e9fb 100644\n--- a/builtin/patch-id.c\n+++ b/builtin/patch-id.c\n@@ -1,3 +1,4 @@\n+#include \"git-compat-util.h\"\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"diff.h\"\n@@ -29,14 +30,16 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n {\n \tstatic const char digits[] = \"0123456789\";\n \tconst char *q, *r;\n+\tchar *endp;\n \tint n;\n \n \tq = p + 4;\n \tn = strspn(q, digits);\n \tif (q[n] == ',') {\n \t\tq += n + 1;\n-\t\t*p_before = atoi(q);\n-\t\tn = strspn(q, digits);\n+\t\tif (strtoi_with_tail(q, 10, p_before, &endp) != 0)\n+\t\t\treturn 0;\n+\t\tn = endp - q;\n \t} else {\n \t\t*p_before = 1;\n \t}\n@@ -48,8 +51,9 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n \tn = strspn(r, digits);\n \tif (r[n] == ',') {\n \t\tr += n + 1;\n-\t\t*p_after = atoi(r);\n-\t\tn = strspn(r, digits);\n+\t\tif (strtoi_with_tail(r, 10, p_after, &endp) != 0)\n+\t\t\treturn 0;\n+\t\tn = endp - r;\n \t} else {\n \t\t*p_after = 1;\n \t}\n-- \ngitgitgadget\n"},{"id":"487573","messageId":"xmqqle87e8nx.fsf@gitster.g","threadId":"60779","inReplyTo":"f09b0838f049de3e4b8cc6adc6cd4492bac7e967.1706416952.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v5 1/2] git-compat-util: add strtoi_with_tail()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-30T04:40:02Z","receivedAt":"2024-01-30T04:40:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Mohit Marathe via GitGitGadget\" writes:\n\n> From: Mohit Marathe \n>\n> This function is an updated version of strtol_i() function. It will\n> give more control to handle parsing of the characters after the\n> numbers and better error handling while parsing numbers.\n>\n> Signed-off-by: Mohit Marathe \n> ---\n> git-compat-util.h | 23 +++++++++++++++++++++++\n> 1 file changed, 23 insertions(+)\n>\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 7c2a6538e5a..c576b1b104f 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -1309,6 +1309,29 @@ static inline int strtol_i(char const *s, int base, int *result)\n> \treturn 0;\n> }\n\nAre we leaving the original one above? Shouldn't this step instead\nremove it, as strtol_i() is now a C preprocessor macro as seen below?\n\n> +#define strtol_i(s,b,r) strtoi_with_tail((s), (b), (r), NULL)\n> +static inline int strtoi_with_tail(char const *s, int base, int *result, char **endp)\n> +{\n> +\tlong ul;\n> +\tchar *dummy = NULL;\n> +\n> +\tif (!endp)\n> +\t\tendp = &dummy;\n> +\terrno = 0;\n> +\tul = strtol(s, endp, base);\n> +\tif (errno ||\n> +\t /*\n> +\t * if we are told to parse to the end of the string by\n> +\t * passing NULL to endp, it is an error to have any\n> +\t * remaining character after the digits.\n> +\t */\n> +\t (dummy && *dummy) ||\n> +\t *endp == s || (int) ul != ul)\n> +\t\treturn -1;\n> +\t*result = ul;\n> +\treturn 0;\n> +}\n> +\n> void git_stable_qsort(void *base, size_t nmemb, size_t size,\n> \t\t int(*compar)(const void *, const void *));\n> #ifdef INTERNAL_QSORT\n"},{"id":"487883","messageId":"pull.1646.v6.git.1707025718.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v5.git.1706416952.gitgitgadget@gmail.com","subject":"[PATCH v6 0/2] Replace atoi() with strtoi_with_tail()","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-02-04T05:48:36Z","receivedAt":"2024-02-04T05:48:42Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"Hello,\n\nThis patch series replaces atoi() with an updated version of strtol_i()\ncalled strtoi_with_tail (Credits: Junio C Hamano). The reasoning behind this\nis to improve error handling by not allowing non-numerical characters in the\nhunk header (which might happen in case of a corrupt patch, although\nrarely).\n\nThere is still a change to be made, as Junio says: \"A corrupt patch may be\ngetting a nonsense patch-ID with the current code and hopefully is not\nmatching other patches that are not corrupt, but with such a change, a\ncorrupt patch may not be getting any patch-ID and a loop that computes\npatch-ID for many files and try to match them up might need to be rewritten\nto take the new failure case into account.\" I'm not sure where this change\nneeds to me made (maybe get_one_patchid()?). It would be great if anyone\ncould point me to the correct place.\n\nThanks, Mohit Marathe\n\nMohit Marathe (2):\n git-compat-util: add strtoi_with_tail()\n patch-id: replace `atoi()` with `strtoi_with_tail`\n\n builtin/patch-id.c | 12 ++++++++----\n git-compat-util.h | 18 ++++++++++++++----\n 2 files changed, 22 insertions(+), 8 deletions(-)\n\n\nbase-commit: 2a540e432fe5dff3cfa9d3bf7ca56db2ad12ebb9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1646%2Fmohit-marathe%2Fupdate-strtol_i-v6\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1646/mohit-marathe/update-strtol_i-v6\nPull-Request: https://github.com/gitgitgadget/git/pull/1646\n\nRange-diff vs v5:\n\n 1: f09b0838f04 ! 1: 98e516a7be7 git-compat-util: add strtoi_with_tail()\n @@ Commit message\n Signed-off-by: Mohit Marathe \n \n ## git-compat-util.h ##\n -@@ git-compat-util.h: static inline int strtol_i(char const *s, int base, int *result)\n +@@ git-compat-util.h: static inline int strtoul_ui(char const *s, int base, unsigned int *result)\n \treturn 0;\n }\n \n +-static inline int strtol_i(char const *s, int base, int *result)\n +#define strtol_i(s,b,r) strtoi_with_tail((s), (b), (r), NULL)\n +static inline int strtoi_with_tail(char const *s, int base, int *result, char **endp)\n -+{\n -+\tlong ul;\n + {\n + \tlong ul;\n +-\tchar *p;\n +\tchar *dummy = NULL;\n -+\n + \n +\tif (!endp)\n +\t\tendp = &dummy;\n -+\terrno = 0;\n + \terrno = 0;\n +-\tul = strtol(s, &p, base);\n +-\tif (errno || *p || p == s || (int) ul != ul)\n +\tul = strtol(s, endp, base);\n +\tif (errno ||\n +\t /*\n @@ git-compat-util.h: static inline int strtol_i(char const *s, int base, int *resu\n +\t */\n +\t (dummy && *dummy) ||\n +\t *endp == s || (int) ul != ul)\n -+\t\treturn -1;\n -+\t*result = ul;\n -+\treturn 0;\n -+}\n -+\n - void git_stable_qsort(void *base, size_t nmemb, size_t size,\n - \t\t int(*compar)(const void *, const void *));\n - #ifdef INTERNAL_QSORT\n + \t\treturn -1;\n + \t*result = ul;\n + \treturn 0;\n 2: ee8f4ae991d = 2: 858d6f94e79 patch-id: replace `atoi()` with `strtoi_with_tail`\n\n-- \ngitgitgadget\n"},{"id":"487884","messageId":"98e516a7be777f2697073e64b28c293e7a8a0011.1707025719.git.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v6.git.1707025718.gitgitgadget@gmail.com","subject":"[PATCH v6 1/2] git-compat-util: add strtoi_with_tail()","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-02-04T05:48:37Z","receivedAt":"2024-02-04T05:48:42Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"From: Mohit Marathe \n\nThis function is an updated version of strtol_i() function. It will\ngive more control to handle parsing of the characters after the\nnumbers and better error handling while parsing numbers.\n\nSigned-off-by: Mohit Marathe \n---\n git-compat-util.h | 18 ++++++++++++++----\n 1 file changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 7c2a6538e5a..59256a441de 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -1296,14 +1296,24 @@ static inline int strtoul_ui(char const *s, int base, unsigned int *result)\n \treturn 0;\n }\n \n-static inline int strtol_i(char const *s, int base, int *result)\n+#define strtol_i(s,b,r) strtoi_with_tail((s), (b), (r), NULL)\n+static inline int strtoi_with_tail(char const *s, int base, int *result, char **endp)\n {\n \tlong ul;\n-\tchar *p;\n+\tchar *dummy = NULL;\n \n+\tif (!endp)\n+\t\tendp = &dummy;\n \terrno = 0;\n-\tul = strtol(s, &p, base);\n-\tif (errno || *p || p == s || (int) ul != ul)\n+\tul = strtol(s, endp, base);\n+\tif (errno ||\n+\t /*\n+\t * if we are told to parse to the end of the string by\n+\t * passing NULL to endp, it is an error to have any\n+\t * remaining character after the digits.\n+\t */\n+\t (dummy && *dummy) ||\n+\t *endp == s || (int) ul != ul)\n \t\treturn -1;\n \t*result = ul;\n \treturn 0;\n-- \ngitgitgadget\n\n"},{"id":"487885","messageId":"858d6f94e79ba388267cc24f43ebf68998cd56a8.1707025719.git.gitgitgadget@gmail.com","threadId":"60779","inReplyTo":"pull.1646.v6.git.1707025718.gitgitgadget@gmail.com","subject":"[PATCH v6 2/2] patch-id: replace `atoi()` with `strtoi_with_tail`","fromName":"Mohit Marathe via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-02-04T05:48:38Z","receivedAt":"2024-02-04T05:48:44Z","isPatch":true,"sender":{"key":"mohitmarathe@proton.me","avatar":"https://gravatar.com/avatar/3bf18f96e6d36cef960077143b9880e8b22468d960da18bf0b34c4e87d4df7e9?d=mp&s=160"},"body":"From: Mohit Marathe \n\nThe change is made to improve the error-handling capabilities\nduring the conversion of string to integers. The\n`strtoi_with_tail` function offers a more robust mechanism for\nconverting strings to integers by providing enhanced error\ndetection. Unlike `atoi`, `strtoi_with_tail` allows the code to\ndifferentiate between a valid conversion and an invalid one,\noffering better resilience against potential issues such as\nreading hunk header of a corrupted patch.\n\nSigned-off-by: Mohit Marathe \n---\n builtin/patch-id.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 3894d2b9706..4e9a301e9fb 100644\n--- a/builtin/patch-id.c\n+++ b/builtin/patch-id.c\n@@ -1,3 +1,4 @@\n+#include \"git-compat-util.h\"\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"diff.h\"\n@@ -29,14 +30,16 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n {\n \tstatic const char digits[] = \"0123456789\";\n \tconst char *q, *r;\n+\tchar *endp;\n \tint n;\n \n \tq = p + 4;\n \tn = strspn(q, digits);\n \tif (q[n] == ',') {\n \t\tq += n + 1;\n-\t\t*p_before = atoi(q);\n-\t\tn = strspn(q, digits);\n+\t\tif (strtoi_with_tail(q, 10, p_before, &endp) != 0)\n+\t\t\treturn 0;\n+\t\tn = endp - q;\n \t} else {\n \t\t*p_before = 1;\n \t}\n@@ -48,8 +51,9 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n \tn = strspn(r, digits);\n \tif (r[n] == ',') {\n \t\tr += n + 1;\n-\t\t*p_after = atoi(r);\n-\t\tn = strspn(r, digits);\n+\t\tif (strtoi_with_tail(r, 10, p_after, &endp) != 0)\n+\t\t\treturn 0;\n+\t\tn = endp - r;\n \t} else {\n \t\t*p_after = 1;\n \t}\n-- \ngitgitgadget\n"}]}