[v2,0/2] Replace atoi() with strtol_i_updated()

Message

Jean-Noël Avila via GitGitGadget Jan. 24, 2024, 6:32 a.m. UTC

Hello,

This patch series replaces atoi() with an updated version of strtol_i()
called strtol_i_updated (Credits: Junio C Hamano). The reasoning behind this
is to improve error handling by not allowing non-numerical characters in the
hunk header (which might happen in case of a corrupt patch, although
rarely).

There is still a change to be made, as Junio says: "A corrupt patch may be
getting a nonsense patch-ID with the current code and hopefully is not
matching other patches that are not corrupt, but with such a change, a
corrupt patch may not be getting any patch-ID and a loop that computes
patch-ID for many files and try to match them up might need to be rewritten
to take the new failure case into account." I'm not sure where this change
needs to me made (maybe get_one_patchid()?). It would be great if anyone
could point me to the correct place.

Thanks, Mohit Marathe

Mohit Marathe (2):
  git-compat-util: add strtol_i_updated()
  patch-id: replace `atoi()` with `strtol_i_updated()`

 builtin/patch-id.c |  8 ++++++--
 git-compat-util.h  | 23 +++++++++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)


base-commit: e02ecfcc534e2021aae29077a958dd11c3897e4c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1646%2Fmohit-marathe%2Fupdate-strtol_i-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1646/mohit-marathe/update-strtol_i-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1646

Range-diff vs v1:

 1:  4e2b03cdd4f ! 1:  60ea85a701a git-compat-util: add strtol_i2
     @@ Metadata
      Author: Mohit Marathe <[e-mail]>
      
       ## Commit message ##
     -    git-compat-util: add strtol_i2
     +    git-compat-util: add strtol_i_updated()
      
     -    This function is an updated version of strtol_i function. It will
     +    This function is an updated version of strtol_i() function. It will
          give more control to handle parsing of the characters after the
          integer and better error handling while parsing numbers.
      
     @@ git-compat-util.h: static inline int strtol_i(char const *s, int base, int *resu
       	return 0;
       }
       
     -+#define strtol_i(s,b,r) strtol_i2((s), (b), (r), NULL)
     -+static inline int strtol_i2(char const *s, int base, int *result, char **endp)
     ++#define strtol_i(s,b,r) strtol_i_updated((s), (b), (r), NULL)
     ++static inline int strtol_i_updated(char const *s, int base, int *result, char **endp)
      +{
      +	long ul;
      +	char *dummy = NULL;
 2:  1ece724b1ca ! 2:  f3a03d68211 patch-id: replace `atoi()` with `strtol_i2()`
     @@ Metadata
      Author: Mohit Marathe <[e-mail]>
      
       ## Commit message ##
     -    patch-id: replace `atoi()` with `strtol_i2()`
     +    patch-id: replace `atoi()` with `strtol_i_updated()`
      
          The change is made to improve the error-handling capabilities
          during the conversion of string representations to integers.
     -    The `strtol_i2(` function offers a more robust mechanism for
     +    The `strtol_i_updated(` function offers a more robust mechanism for
          converting strings to integers by providing enhanced error
     -    detection. Unlike `atoi(`, `strtol_i2(` allows the code to
     +    detection. Unlike `atoi(`, `strtol_i_updated(` 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.
     @@ builtin/patch-id.c: static int scan_hunk_header(const char *p, int *p_before, in
       	if (q[n] == ',') {
       		q += n + 1;
      -		*p_before = atoi(q);
     -+		if (strtol_i2(q, 10, p_before, &endp) != 0)
     ++		if (strtol_i_updated(q, 10, p_before, &endp) != 0)
      +			return 0;
       		n = strspn(q, digits);
       	} else {
       		*p_before = 1;
     - 	}
     - 
     --	if (n == 0 || q[n] != ' ' || q[n+1] != '+')
     -+	if (q[n] != ' ' || q[n+1] != '+')
     - 		return 0;
     - 
     - 	r = q + n + 2;
     +@@ builtin/patch-id.c: 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 (strtol_i2(r, 10, p_after, &endp) != 0)
     ++		if (strtol_i_updated(r, 10, p_after, &endp) != 0)
      +			return 0;
     + 		n = strspn(r, digits);
       	} else {
       		*p_after = 1;
     - 	}
     --	if (n == 0)
     --		return 0;
     --
     - 	return 1;
     - }
     -