Re: [PATCH 2/3] config: require at least one digit when parsing numbers
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 21, 2022, 18:19 UTC
- Message-ID
- <xmqqwn8tjazb.fsf@gitster.g>
- In-Reply-To
- <cd753602e48a2faa0d59edca2f6fab0fe753f0f6.1666359915.git.gitgitgadget@gmail.com>
"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 25 quoted lines
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> If the input to strtoimax() or strtoumax() does not contain any digits
> then they return zero and set `end` to point to the start of the input
> string. git_parse_[un]signed() do not check `end` and so fail to return
> an error and instead return a value of zero if the input string is a
> valid units factor without any digits (e.g "k").
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> config.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/config.c b/config.c
> index d5069d4f01d..b7fb68026d8 100644
> --- a/config.c
> +++ b/config.c
> @@ -1167,6 +1167,10 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
> val = strtoimax(value, &end, 0);
> if (errno == ERANGE)
> return 0;
> + if (end == value) {
> + errno = EINVAL;
> + return 0;
> + }This means well, but doesn't strto*() family of functions silently ignore leading blanks, e.g.
l = strtol(" 432k", &end, 0);
... l == 432, *end = k ...If you really want to reject a string with no number before the optional unit, end at this point may not match value. With " k" as input, value would point at the space at the beginning, and end would point at 'k'.
It does not look _too_ bad if we just let such an empty string through and interpreted it as zero. Is that a problem? Who are we trying to help?
Show 14 quoted lines
> factor = get_unit_factor(end);
> if (!factor) {
> errno = EINVAL;
> @@ -1202,6 +1206,10 @@ static int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max)
> val = strtoumax(value, &end, 0);
> if (errno == ERANGE)
> return 0;
> + if (end == value) {
> + errno = EINVAL;
> + return 0;
> + }
> factor = get_unit_factor(end);
> if (!factor) {
> errno = EINVAL;