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 <peff@xxxxxxxx> --- 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html