Thread (1 message) 1 message, 1 author, 2016-06-15

Re: Git compile warnings (under mac/clang)

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:03:40

Jeff King [off-list ref] writes:
But of all the options outlined, I think I'd much rather just see an
assert() for something that should never happen, rather than mixing it
into the logic.
Surely.
In that vein, one thing that puzzles me is that the current code looks
like:

  if (options->msg_severity && msg_id >= 0 && msg_id < FSCK_MSG_MAX)
	  severity = options->msg_severity[msg_id];
  else {
	  severity = msg_id_info[msg_id].severity;
	  ...
  }

So if the severity override list given by "options" exists, _and_ if we
are in the enum range, then we use that. Otherwise, we dereference the
global list. But wouldn't an out-of-range condition have the exact same
problem dereferencing that global list?

IOW, should this really be:

  if (msg_id < 0 || msg_id >= FSCK_MSG_MAX)
	die("BUG: broken enum");

  if (options->msg_severity)
	severity = options->msg_severity[msg_id];
  else
	severity = msg_id_info[msg_id].severity;

? And then you can spell that first part as assert(), which I suspect
(but did not test) may shut up clang's warnings.
Sounds like a sensible fix to me.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help