[PATCH v2] strtoul_ui: reject negative values
- From
Matthieu Moy <matthieu.moy@imag.fr>
- Date
- Sep 17, 2015, 16:28 UTC
- Message-ID
- <1442507313-13028-1-git-send-email-Matthieu.Moy@imag.fr>
- In-Reply-To
- <xmqqpp1hj9yu.fsf@gitster.mtv.corp.google.com>
strtoul_ui uses strtoul to get a long unsigned, then checks that casting to unsigned does not lose information and return the casted value.
On 64 bits architecture, checking that the cast does not change the value catches most errors, but when sizeof(int) == sizeof(long) (e.g. i386), the check does nothing. Unfortunately, strtoul silently accepts negative values, and as a result strtoul_ui("-1", ...) raised no error.
This patch catches negative values before it's too late, i.e. before calling strtoul.
Reported-by: Max Kirillov <max@max630.net> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> --- Junio C Hamano <gitster@pobox.com> writes:
Show 8 quoted lines
> Matthieu Moy <Matthieu.Moy@imag.fr> writes: > >> This patch catches negative values before it's too late, i.e. before >> calling strtoul. We still silently accept very large integers that wrap >> to a valid "unsigned int". > > Is the last statement correct? A very large long uint that wrap to > uint would not fit in long uint and you would get ERANGE, no?
Indeed. strtoul happily accepts negative values, but not overly large ones.
I removed the sentence from the message. Actually, I think we are now accepting exactly the right interval of values.
>> It should be merged before Kartik's series (or inserted at the start >> of the series) so that we get the fix before the test breakage. > > Which one of his series?
kn/for-each-tag, which uses strtoul_ui for align:<num> and content:lines=<num>.
git-compat-util.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h index f649e81..1df82fa 100644 --- 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; ul = strtoul(s, &p, base); if (errno || *p || p == s || (unsigned int) ul != ul) return -1;
-- 2.5.0.402.g8854c44