Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 17, 2015, 18:48 UTC
Message-ID: <xmqqk2yfo3y0.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1426608016-2978-1-git-send-email-mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

Show 7 quoted lines

> When I looked around, I found scores of sites that call atoi(),
> strtoul(), and strtol() carelessly. And it's understandable. Calling
> any of these functions safely is so much work as to be completely
> impractical in day-to-day code.
>
> git-compat-util.h has two functions, strtoul_ui() and strtol_i(), that
> try to make parsing integers a little bit easier.

Exactly. They were introduced to prevent sloppy callers from passing NULL to the &end parameter to underlying strtoul(3). The first thing that came to my mind while reading your message up to this point was "why not use them more, tighten them, adding different variants if necessary, instead of introducing an entirely new set of functions in a new namespace?"

For example:

Show 10 quoted lines

> * Am I making callers too strict? In many cases where a positive
>   integer is expected (e.g., "--abbrev=<num>"), I have been replacing
>   code like
>
>       result = strtoul(s, NULL, 10);
>
>   with
>
>       if (convert_i(s, 10, &result))
>               die("...");

I think strictness would be good and those who did "--abbrev=' 20'" deserve what they get from the additional strictness, but

	if (strtol_i(s, 10, &result))

would have been more familiar.