On Fri, Jan 23, 2015 at 07:46:36PM +0100, Johannes Schindelin wrote:

> > ? And then you can spell that first part as assert(), which I suspect
> > (but did not test) may shut up clang's warnings.
> 
> To be quite honest, I assumed that Git's source code was
> assert()-free. But I was wrong! So I'll go with that solution; it is
> by far the nicest one IMHO.

OK, here it is as a patch on top of js/fsck-opt. Please feel free to
squash rather than leaving it separate.

I tested with clang-3.6, and it seems to make the warning go away.

-- >8 --
Subject: [PATCH] fsck_msg_severity: range-check enum with assert()

An enum is passed into the function, which we use to index a
fixed-size array. We double-check that the enum is within
range as a defensive measure. However, there are two
problems with this:

  1. If it's not in range, we then use it to index another
     array of the same size. Which will have the same problem.
     We should probably die instead, as this condition
     should not ever happen.

  2. The bottom end of our range check is tautological
     according to clang, which generates a warning. Clang is
     not _wrong_, but the point is that we are trying to be
     defensive against something that should never happen
     (i.e., somebody abusing the enum).

We can solve both by switching to a separate assert().

Signed-off-by: Jeff King <p...@peff.net>
---
 fsck.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fsck.c b/fsck.c
index 15cb8bd..53c0849 100644
--- a/fsck.c
+++ b/fsck.c
@@ -107,7 +107,9 @@ static int fsck_msg_severity(enum fsck_msg_id msg_id,
 {
        int severity;
 
-       if (options->msg_severity && msg_id >= 0 && msg_id < FSCK_MSG_MAX)
+       assert(msg_id >= 0 && msg_id < FSCK_MSG_MAX);
+
+       if (options->msg_severity)
                severity = options->msg_severity[msg_id];
        else {
                severity = msg_id_info[msg_id].severity;
-- 
2.3.0.rc1.287.g761fd19

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to