Marc Branchaud <[email protected]> writes:

>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -814,6 +814,9 @@ static inline int strtoul_ui(char const *s, int base, 
>> unsigned int *result)
>>      char *p;
>>  
>>      errno = 0;
>> +    /* negative values would be accepted by strtoul */
>> +    if (strchr(s, '-'))
>> +            return -1;
>
> I think this is broken, in that it doesn't match strtoul's normal behaviour,
> for strings like "1234-5678", no?

The goal here is just to read a positive integer value. Rejecting
"1234-5678" is indeed a good thing. We already rejected it before my
patch by checking for p (AKA endptr for strtoul), as you noted below.

> The test also doesn't work if the string has leading whitespace ("
> -5").

Why? It rejects any string that contain the character '-', regardless of
trailing spaces.

>>      ul = strtoul(s, &p, base);
>>      if (errno || *p || p == s || (unsigned int) ul != ul)
>>              return -1;
>
> Hmm, but we check *p here, so IIUC it's an error if the string has any
> trailing non-digits.  Weird.

strtoul_ui is more defensive than strtoul, by design.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to