git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: Junio C Hamano <gitster@pobox.com>,
	Stefan Beller <sbeller@google.com>,
	Michael Blume <blume.mike@gmail.com>,
	peter@lekensteyn.nl, eungjun.yi@navercorp.com,
	Git List <git@vger.kernel.org>
Subject: Re: Git compile warnings (under mac/clang)
Date: Fri, 23 Jan 2015 13:55:58 -0500	[thread overview]
Message-ID: <20150123185558.GC32191@peff.net> (raw)
In-Reply-To: <321a67762d18795b743b242618950849@www.dscho.org>

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;
-- 
2.3.0.rc1.287.g761fd19

  reply	other threads:[~2015-01-23 18:56 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-22 19:43 Git compile warnings (under mac/clang) Michael Blume
2015-01-22 19:59 ` Stefan Beller
2015-01-22 21:19   ` Peter Wu
2015-01-22 21:20   ` Johannes Schindelin
2015-01-22 22:01     ` Jeff King
2015-01-23 11:48       ` Johannes Schindelin
2015-01-23 12:23         ` Jeff King
2015-01-23 12:38           ` Johannes Schindelin
2015-01-23 13:30             ` Jeff King
2015-01-23 18:07               ` Junio C Hamano
2015-01-23 18:37                 ` Jeff King
2015-01-23 18:46                   ` Johannes Schindelin
2015-01-23 18:55                     ` Jeff King [this message]
2015-01-23 19:20                       ` Johannes Schindelin
2015-01-23 18:48                   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150123185558.GC32191@peff.net \
    --to=peff@peff.net \
    --cc=blume.mike@gmail.com \
    --cc=eungjun.yi@navercorp.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peter@lekensteyn.nl \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).