Thread (1 message) 1 message, 1 author, 2016-06-15

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:04:16

Michael Haggerty [off-list ref] 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: optarg()
This one I know is from "if not a digit we know it is not a number";
it is not an attempt to say "we must forbid numbers to be spelled
with '+'", but more about "we do not need it and this is easier to
code without a full fledged str-to-num helper API" sloppiness.
    builtin/apply.c: parse_num()
This parses "@@ -l,k +m,n @@@" after stripping the punctuation
around the numbers, so this is a valid reason why you would want an
optional feature "CANNOT_HAVE_SIGN" in the str-to-num parser and use
it.
    maybe date.c: match_multi_number()
The approxidate callers parse random garbage input and attempt to
make best sense out of it, so they tend to try limitting the damage
caused by incorrect guesses by insisting "we do not consider +0 to
be zero" and such.  So this is a valid reason why you would want an
optional feature "CANNOT_HAVE_SIGN" in the str-to-num parser and use
it.
I'm coming around to an alternate plan:

Step 1: write a NUM_DEFAULT combination-of-options that gives the new
functions behavior very like strtol()/strtoul() but without their insane
features.

Step 2: rewrite all callers to use that option (and usually endptr=NULL,
meaning no trailing characters) unless it is manifestly clear that they
are already trying to forbid some other features. This will already
produce the largest benefit: avoiding overflows, missing error checking,
etc.

Steps 3 through ∞: as time allows, rewrite individual callsites to be
stricter where appropriate.

Hopefully steps 1 and 2 will not be too controversial.
All sounds sensible to me.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help