git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Git compile warnings (under mac/clang)

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Jan 23, 2015, 19:20 UTC
Message-ID
<4050d800c45aa32eda029a52f3556dcc@www.dscho.org>
In-Reply-To
<20150123185558.GC32191@peff.net>
Hi Peff,
On 2015-01-23 19:55, Jeff King wrote:
Show 55 quoted lines
> 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@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;
I also ended up with that solution!

Thanks! Dscho

Previous: Jeff KingNext: Junio C Hamano
Message 14 of 15 in “Git compile warnings (under mac/clang)”
  1. Michael BlumeJan 22, 2015
  2. Stefan BellerJan 22, 2015
  3. Peter WuJan 22, 2015
  4. Johannes SchindelinJan 22, 2015
  5. Jeff KingJan 22, 2015
  6. Johannes SchindelinJan 23, 2015
  7. Jeff KingJan 23, 2015
  8. Johannes SchindelinJan 23, 2015
  9. Jeff KingJan 23, 2015
  10. Junio C HamanoJan 23, 2015
  11. Jeff KingJan 23, 2015
  12. Johannes SchindelinJan 23, 2015
  13. Jeff KingJan 23, 2015
  14. Johannes SchindelinJan 23, 2015
  15. Junio C HamanoJan 23, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.