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

From: Michael Haggerty <[email-masked]>
Date: Wed, 18 Mar 2015 10:47:40 +0100

On 03/18/2015 12:05 AM, Duy Nguyen wrote:
> On Tue, Mar 17, 2015 at 11:00 PM, Michael Haggerty <[email-masked]> 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
>>   handle_revision_opt(): use skip_prefix() in many places
>>   handle_revision_opt(): use convert_i() when handling "-<digit>"
>>   strtoul_ui(), strtol_i(): remove functions
>>   handle_revision_opt(): use convert_ui() when handling "--abbrev="
>>   builtin_diff(): detect errors when parsing --unified argument
>>   opt_arg(): val is always non-NULL
>>   opt_arg(): use convert_i() in implementation
>>   opt_arg(): report errors parsing option values
>>   opt_arg(): simplify pointer handling
>>   diff_opt_parse(): use convert_i() when handling "-l<num>"
>>   diff_opt_parse(): use convert_i() when handling --abbrev=<num>
> 
> 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 in base 10, plus sign is not accepted.

If the general direction of this patch series is accepted, I'll
gradually try to go through the codebase, replacing *all* integer
parsing with these functions. So there's no need to request particular
callers of strtol()/strtoul() to be converted; I'll get to them all
sooner or later (I hope).

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" patch series.

By the way, some other packet line parsing code in that file doesn't
verify that there are no trailing characters on the lines that they
process. That might be another thing that should be tightened up.

Michael

[1] https://github.com/mhagger/git.git, branch "upload-pack-numparse"

-- 
Michael Haggerty
[email-masked]