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

2015-03-25 Thread Michael Haggerty
On 03/24/2015 05:49 PM, René Scharfe wrote: Am 24.03.2015 um 17:06 schrieb Michael Haggerty: Parsing numbers is not rocket science, but there are a lot of pitfalls, especially around overflow. It's even harder to write such code via macros and the result is less readable. This patch series

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

2015-03-25 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes: [jc: dropped a non-working address from Cc list] I wasn't aware of strtonum; thanks for the reference. It has an untraditional interface. Their willingness to sacrifice half of the unsigned range and requirement that the user specify minval and

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

2015-03-24 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes: It is easy to allow --abbrev=+7; I would just need to add NUM_PLUS to those call sites. Should I do so? The more relevant question to ask from my point of view is why you need to add NUM_PLUS to enable it. What valid reason do you have to forbid

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

2015-03-24 Thread Michael Haggerty
On 03/19/2015 06:26 AM, Jeff King wrote: On Tue, Mar 17, 2015 at 05:00:02PM +0100, Michael Haggerty wrote: My main questions: * Do people like the API? My main goal was to make these functions as painless as possible to use correctly, because there are so many call sites. * Is it too

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

2015-03-24 Thread Michael Haggerty
On 03/19/2015 08:32 AM, Junio C Hamano wrote: Jeff King p...@peff.net writes: I wonder how much of the boilerplate in the parse_* functions could be factored out to use a uintmax_t, with the caller just providing the range. That would make it easier to add new types like off_t, and possibly

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

2015-03-24 Thread René Scharfe
Am 24.03.2015 um 17:06 schrieb Michael Haggerty: Parsing numbers is not rocket science, but there are a lot of pitfalls, especially around overflow. It's even harder to write such code via macros and the result is less readable. This patch series is mostly about finding a reasonable API and

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

2015-03-24 Thread Michael Haggerty
On 03/24/2015 04:58 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: It is easy to allow --abbrev=+7; I would just need to add NUM_PLUS to those call sites. Should I do so? The more relevant question to ask from my point of view is why you need to add NUM_PLUS to

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

2015-03-24 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes: Michael Haggerty mhag...@alum.mit.edu writes: It is easy to allow --abbrev=+7; I would just need to add NUM_PLUS to those call sites. Should I do so? The more relevant question to ask from my point of view is why you need to add NUM_PLUS to enable

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

2015-03-24 Thread Michael Haggerty
On 03/19/2015 07:22 AM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: * It allows leading whitespace. This might be blessing in disguise. Our friends on MacOS may be relying on that git cmd --abbrev=$(wc -c foo) to work as expected, even though their wc

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

2015-03-24 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes: Regarding specifically allowing/disallowing a leading '+': I saw a couple of callsites that explicitly check that the first character is a digit before calling strtol(). I assumed that is to disallow sign characters [1]. For example, diff.c:

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

2015-03-19 Thread Junio C Hamano
Jeff King p...@peff.net writes: I wonder how much of the boilerplate in the parse_* functions could be factored out to use a uintmax_t, with the caller just providing the range. That would make it easier to add new types like off_t, and possibly even constrained types (e.g., an integer from 0

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

2015-03-19 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes: Here were my thoughts: * I wanted to change the interface to look less like strtol()/strtoul(), so it seemed appropriate to make the names more dissimilar. One reason I prefer the names in the compat-util is that it makes it clear that what

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

2015-03-19 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes: * It allows leading whitespace. This might be blessing in disguise. Our friends on MacOS may be relying on that git cmd --abbrev=$(wc -c foo) to work as expected, even though their wc gives leading spaces, for example. * It allows arbitrary

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

2015-03-19 Thread Junio C Hamano
Jeff King p...@peff.net writes: 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

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

2015-03-18 Thread Duy Nguyen
On Wed, Mar 18, 2015 at 4:47 PM, Michael Haggerty mhag...@alum.mit.edu wrote: Thank you for doing it. I was about to write another number parser and you did it :D Maybe you can add another patch to convert the only strtol in upload-pack.c to parse_ui. This place should accept positive number

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

2015-03-18 Thread Michael Haggerty
On 03/18/2015 11:03 AM, Jeff King wrote: On Wed, Mar 18, 2015 at 10:47:40AM +0100, Michael Haggerty wrote: But in case you have some reason that you want upload-pack.c to be converted right away, I just pushed that change (plus some related cleanups) to my GitHub repo [1]. The branch depends

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

2015-03-18 Thread Michael Haggerty
On 03/18/2015 12:05 AM, Duy Nguyen wrote: On Tue, Mar 17, 2015 at 11:00 PM, Michael Haggerty mhag...@alum.mit.edu wrote: Michael Haggerty (14): numparse: new module for parsing integral numbers cacheinfo_callback(): use convert_ui() when handling --cacheinfo write_subdirectory(): use

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

2015-03-18 Thread Jeff King
On Wed, Mar 18, 2015 at 10:47:40AM +0100, Michael Haggerty wrote: But in case you have some reason that you want upload-pack.c to be converted right away, I just pushed that change (plus some related cleanups) to my GitHub repo [1]. The branch depends only on the first patch of the numparse

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

2015-03-18 Thread Jeff King
On Tue, Mar 17, 2015 at 05:00:02PM +0100, Michael Haggerty wrote: My main questions: * Do people like the API? My main goal was to make these functions as painless as possible to use correctly, because there are so many call sites. * Is it too gimmicky to encode the base together

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

2015-03-17 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes: 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.

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

2015-03-17 Thread Duy Nguyen
On Tue, Mar 17, 2015 at 11:00 PM, Michael Haggerty mhag...@alum.mit.edu wrote: Michael Haggerty (14): numparse: new module for parsing integral numbers cacheinfo_callback(): use convert_ui() when handling --cacheinfo write_subdirectory(): use convert_ui() for parsing mode

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

2015-03-17 Thread Michael Haggerty
On 03/17/2015 07:48 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: 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

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

2015-03-17 Thread Michael Haggerty
Sorry for the long email. Feel free to skip over the background info and continue reading at My solution below. This odyssey started with a typo: $ git shortlog -snl v2.2.0..v2.3.0 14795 Junio C Hamano 1658 Jeff King 1400 Shawn O. Pearce 1109 Linus