Re: Git compile warnings (under mac/clang)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]