From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Michael Haggerty <mhagger@alum.mit.edu>,
Jeff King <peff@peff.org>,
git@vger.kernel.org
Subject: Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
Date: Wed, 18 Mar 2015 23:41:14 -0700 [thread overview]
Message-ID: <xmqqoanpjxpx.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20150319052620.GA30645@peff.net> (Jeff King's message of "Thu, 19 Mar 2015 01:26:20 -0400")
Jeff King <peff@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 the general reason for
> doing octal parsing. 100000644 does not overflow an int, but it is
> hardly a reasonable mode.
True.
> 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.
next prev parent reply other threads:[~2015-03-19 6:41 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-17 16:00 [PATCH 00/14] numparse module: systematically tighten up integer parsing Michael Haggerty
2015-03-17 16:00 ` [PATCH 01/14] numparse: new module for parsing integral numbers Michael Haggerty
2015-03-18 18:27 ` Eric Sunshine
2015-03-18 22:47 ` Michael Haggerty
2015-03-20 8:54 ` Eric Sunshine
2015-03-20 17:51 ` Junio C Hamano
2015-03-17 16:00 ` [PATCH 02/14] cacheinfo_callback(): use convert_ui() when handling "--cacheinfo" Michael Haggerty
2015-03-17 16:00 ` [PATCH 03/14] write_subdirectory(): use convert_ui() for parsing mode Michael Haggerty
2015-03-17 16:00 ` [PATCH 04/14] handle_revision_opt(): use skip_prefix() in many places Michael Haggerty
2015-03-17 16:00 ` [PATCH 05/14] handle_revision_opt(): use convert_i() when handling "-<digit>" Michael Haggerty
2015-03-19 6:34 ` Junio C Hamano
2015-03-17 16:00 ` [PATCH 06/14] strtoul_ui(), strtol_i(): remove functions Michael Haggerty
2015-03-17 16:00 ` [PATCH 07/14] handle_revision_opt(): use convert_ui() when handling "--abbrev=" Michael Haggerty
2015-03-17 16:00 ` [PATCH 08/14] builtin_diff(): detect errors when parsing --unified argument Michael Haggerty
2015-03-17 16:00 ` [PATCH 09/14] opt_arg(): val is always non-NULL Michael Haggerty
2015-03-17 16:00 ` [PATCH 10/14] opt_arg(): use convert_i() in implementation Michael Haggerty
2015-03-17 16:00 ` [PATCH 11/14] opt_arg(): report errors parsing option values Michael Haggerty
2015-03-17 16:00 ` [PATCH 12/14] opt_arg(): simplify pointer handling Michael Haggerty
2015-03-17 16:00 ` [PATCH 13/14] diff_opt_parse(): use convert_i() when handling "-l<num>" Michael Haggerty
2015-03-17 16:00 ` [PATCH 14/14] diff_opt_parse(): use convert_i() when handling --abbrev=<num> Michael Haggerty
2015-03-19 6:37 ` Junio C Hamano
2015-03-17 18:48 ` [PATCH 00/14] numparse module: systematically tighten up integer parsing Junio C Hamano
2015-03-17 19:46 ` Michael Haggerty
2015-03-19 6:31 ` Junio C Hamano
2015-03-17 23:05 ` Duy Nguyen
2015-03-18 9:47 ` Michael Haggerty
2015-03-18 9:58 ` Duy Nguyen
2015-03-18 10:03 ` Jeff King
2015-03-18 10:20 ` Michael Haggerty
2015-03-19 5:26 ` Jeff King
2015-03-19 6:41 ` Junio C Hamano [this message]
2015-03-19 7:32 ` Junio C Hamano
2015-03-24 16:06 ` Michael Haggerty
2015-03-24 16:49 ` René Scharfe
2015-03-25 21:14 ` Michael Haggerty
2015-03-25 21:59 ` Junio C Hamano
2015-03-24 15:05 ` Michael Haggerty
2015-03-19 6:22 ` Junio C Hamano
2015-03-24 15:42 ` Michael Haggerty
2015-03-24 15:58 ` Junio C Hamano
2015-03-24 16:09 ` Junio C Hamano
2015-03-24 17:39 ` Michael Haggerty
2015-03-24 18:08 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqoanpjxpx.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=mhagger@alum.mit.edu \
--cc=peff@peff.net \
--cc=peff@peff.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.