[GSoC][RFC] Replace use of atoi() with strtol_i(), as a microproject

From: mohitmarathe@proton.me <mohitmarathe@proton.me>
Date: Jan 8, 2024, 17:34 UTC
Message-ID: <OqD4xQ_p-jcftCbAw0ovvrBztfiuoMGcTonCc0i6x7Ziy-hx3uA-Hoz4-3tfRI39KMj-V5OZIGgOe66b1eyX3YrKZNThMYjjMkn6g4-Ww8c=@proton.me>

Hello,

I'm Mohit, an undergrad from India, and I want to start contributing to the Git project. I have already built Git from source and finished `git psuh` tutorial. And I must say the "Hacking Git" documentation is great (very detailed and maybe exhaustive) and easy to follow. So I also read the topic on "microprojects", and while searching for one, I came across this message: https://public-inbox.org/git/xmqqjzpjsbjl.fsf@gitster.g/. I want to work on this task (if it is not taken up already) as a microproject for GSoC.

Approach: From what I understood, the idea is to *not* allow non-integer characters in the arguments by printing an error message. So we have to replace `atoi` with `strtol_i`, like it is done in this patch: https://public-inbox.org/git/xmqq5y181fx0.fsf_-_@gitster.g/. There are some places where we want to continue to parse after the end of the integer (where `strspn` is used as mentioned by Junio), and based on Junio's suggestion, a new helper can be created like this:

Show 21 quoted lines

> static inline int strtol_i2(char const *s, int base, int *result, char **endp)
> {
> 	long ul;
>         char *dummy = NULL;
> 
>         if (!endp)
> 		endp = &dummy;
> 	errno = 0;
> 	ul = strtol(s, &endp, base);
> 	if (errno ||
> 	    /*
> 	     * if we are told to parse to the end of the string by
> 	     * passing NULL to endp, it is an error to have any
> 	     * remaining character after the digits.
> 	     */
>             (dummy && *dummy) ||
> 	    endp == s || (int) ul != ul)
> 		return -1;
> 	*result = ul;
> 	return 0;
> }

So I searched for all the occurrences of `atoi(` (as suggested by Junio), and I found only two instances (both in `builtin/patch-id.c`) where it is followed by `strspn`. Is it safe to assume that this is the only place where we cannot directly replace `atoi` with `strtol_i`, or should I keep digging?

Also, this seems like a large change due to the number of files involved, while the documentation about the microproject emphasizes keeping the change small. Does it mean a small change per commit or a small change per Pull Request?

Thanks!