Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Nov 7, 2011, 19:49 UTC
- Message-ID
- <20111107194912.GA12469@elie.hsd1.il.comcast.net>
- In-Reply-To
- <1320581184-4557-4-git-send-email-avarab@gmail.com>
Hi,
Show 8 quoted lines
> --- a/grep.c
> +++ b/grep.c
> @@ -327,7 +327,7 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
> for (p = opt->header_list; p; p = p->next) {
> if (p->token != GREP_PATTERN_HEAD)
> die("bug: a non-header pattern in grep header list.");
> - if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)
> + if (GREP_HEADER_FIELD_MAX <= p->field)I imagine the following would be less controversial.
-- >8 -- From: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Subject: grep: get rid of bounds check on "enum grep_header_field" value
The grep_header_field enum is defined as:
enum grep_header_field {
GREP_HEADER_AUTHOR = 0,
GREP_HEADER_COMMITTER
};Git's grep code sanity-checks the value of p->field before using it in order to avoid reading and writing past the end of an array. Unfortunately that test trips up misguided static analyzers like "gcc -Wtype-limits" that do not recognize that C allows this enum to be a signed integer type and an "x < 0" comparison really could fire if someone passed in an uninitialized value.
Let's just drop the check. Luckily git always does initialize p->field correctly, and if it were to stop doing so, then hopefully running the test suite with valgrind would catch it.
Noticed with clang -Wtautological-compare.
[jn: drop the GREP_HEADER_FIELD_MAX <= p->field check, too, for symmetry]
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- grep.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/grep.c b/grep.c index b29d09c7..424c46cd 100644 --- a/grep.c +++ b/grep.c @@ -327,8 +327,6 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt) for (p = opt->header_list; p; p = p->next) { if (p->token != GREP_PATTERN_HEAD) die("bug: a non-header pattern in grep header list."); - if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field) - die("bug: unknown header field %d", p->field); compile_regexp(p, opt); }
-- 1.7.8.rc0