Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 19, 2015, 06:41 UTC
- Message-ID
- <xmqqoanpjxpx.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <20150319052620.GA30645@peff.net>
Jeff King <peff@peff.net> writes:
Show 6 quoted lines
> I wondered if we could do away with the radix entirely. Wouldn't we be > asking for base 10 most of the time? Of course, your first few patches > show octal parsing, but I wonder if we should actually have a separate > parse_mode() for that, since that seems to be the general reason for > doing octal parsing. 100000644 does not overflow an int, but it is > hardly a reasonable mode.
True.
Show 6 quoted lines
> I also wondered if we could get rid of NUM_SIGN in favor of just having > the type imply it (e.g., convert_l would always allow negative numbers, > whereas convert_ul would not). But I suppose there are times when we end > up using an "int" to store an unsigned value for a good reason (e.g., > "-1" is a sentinel value, but we expect only positive values from the > user). So that might be a bad idea.
Yeah, there is a reason why ssize_t exists.
> IMHO most of the tightening happening here is a good thing, and means we > are more likely to notice mistakes rather than silently doing something > stupid.
I do like the general idea of making sure we avoid use of bare atoi() and unchecked use of strtol() and such, and have a set of well defined helper functions to perform the common checks, exactly for the reason you said.