[PATCH v6 2/2] patch-id: replace `atoi()` with `strtoi_with_tail`

[PATCH v6 2/2] patch-id: replace `atoi()` with `strtoi_with_tail`

[Mohit Marathe via GitGitGadget]

From: Mohit Marathe <mohitmarathe23@gmail.com>

The change is made to improve the error-handling capabilities
during the conversion of string to integers. The
`strtoi_with_tail` function offers a more robust mechanism for
converting strings to integers by providing enhanced error
detection. Unlike `atoi`, `strtoi_with_tail` allows the code to
differentiate between a valid conversion and an invalid one,
offering better resilience against potential issues such as
reading hunk header of a corrupted patch.

Signed-off-by: Mohit Marathe <mohitmarathe@proton.me>
---
 builtin/patch-id.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 3894d2b9706..4e9a301e9fb 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -1,3 +1,4 @@
+#include "git-compat-util.h"
 #include "builtin.h"
 #include "config.h"
 #include "diff.h"
@@ -29,14 +30,16 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)
 {
 	static const char digits[] = "0123456789";
 	const char *q, *r;
+	char *endp;
 	int n;
 
 	q = p + 4;
 	n = strspn(q, digits);
 	if (q[n] == ',') {
 		q += n + 1;
-		*p_before = atoi(q);
-		n = strspn(q, digits);
+		if (strtoi_with_tail(q, 10, p_before, &endp) != 0)
+			return 0;
+		n = endp - q;
 	} else {
 		*p_before = 1;
 	}
@@ -48,8 +51,9 @@ 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);
-		n = strspn(r, digits);
+		if (strtoi_with_tail(r, 10, p_after, &endp) != 0)
+			return 0;
+		n = endp - r;
 	} else {
 		*p_after = 1;
 	}
-- 
gitgitgadget

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

[Mohit Marathe]

Hi,

I am writing to inquire about the status of this patch. 

As a contributor, I am curious whether the decision not 
to merge this patch was intentional or if it might have been 
overlooked. Could you kindly provide some clarity on this matter?
I tried finding this on "What's cooking in Git" but couldn't find it.

If there are any specific reasons for not merging the patch, I would 
be grateful if you could share them. Additionally, if there are any 
further corrections needed please do let me know.

Thanks,
Mohit

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

[Junio C Hamano]

Mohit Marathe <mohitmarathe@proton.me> writes:

> I am writing to inquire about the status of this patch. 

Thanks for pinging.

Please be kind to fellow project members by including a URL under
https://lore.kernel.org/git/ to the original discussion.  In general
there needs to be a reason why a patch should be applied, but a lack
of reason why a patch should be applied is good enough reason why a
patch may not have been applied so far.  I cannot offhand recall
what problems the patch had or if there were additional problems in
the patch that was left unaddressed.

Thanks.

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

[Mohit Marathe]

> Mohit Marathe mohitmarathe@proton.me writes:
> 
> > I am writing to inquire about the status of this patch.
> 
> 
> Thanks for pinging.
> 
> Please be kind to fellow project members by including a URL under
> https://lore.kernel.org/git/ to the original discussion. In general

I thought replying in the same thread as that of patch would be enough.
But now that I think about it, threads can get pretty long. My bad,I 
will keep in mind to include the URL of the patch next time.

Just for the record, I was talking about this patch:
https://lore.kernel.org/git/pull.1646.v6.git.1707025718.gitgitgadget
@gmail.com/

> there needs to be a reason why a patch should be applied, but a lack
> of reason why a patch should be applied is good enough reason why a
> patch may not have been applied so far. I cannot offhand recall
> what problems the patch had or if there were additional problems in
> the patch that was left unaddressed.
> 
> Thanks.

This makes perfect sense. Thanks for clarifying.

Thanks.

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

[Junio C Hamano]

Mohit Marathe <mohitmarathe@proton.me> writes:

> I thought replying in the same thread as that of patch would be enough.

Well, if you look at the headers of your message, there only is its
own Message-ID and no In-Reply-To or References, which some/many
MUAs go by.

> Just for the record, I was talking about this patch:
> https://lore.kernel.org/git/pull.1646.v6.git.1707025718.gitgitgadget
> @gmail.com/

Yup, thanks.  I ended up at the same place with subject search ;-)