Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:52:26
Subsystem:
the rest · Maintainer:
Linus Torvalds
Hi,
quoted hunk ↗ jump to hunk
--- 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 <redacted>
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 <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
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