Hi Peff,
On 2015-01-23 19:55, Jeff King wrote:
> 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;
I also ended up with that solution!
Thanks!
Dscho
--
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