{"thread":{"id":"35061","subject":"[PATCH] silence gcc array-bounds warning","startedAt":"2013-10-04T07:52:35Z","lastAt":"2013-10-04T07:52:35Z","messageCount":1,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"228551","messageId":"20131004075234.GA26068@sigill.intra.peff.net","threadId":"35061","inReplyTo":null,"subject":"[PATCH] silence gcc array-bounds warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-04T07:52:35Z","receivedAt":"2013-10-04T07:52:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In shorten_unambiguous_ref, we build and cache a reverse-map of the\nrev-parse rules like this:\n\n  static char **scanf_fmts;\n  static int nr_rules;\n  if (!nr_rules) {\n\t  for (; ref_rev_parse_rules[nr_rules]; nr_rules++)\n\t\t  ... generate scanf_fmts ...\n  }\n\nwhere ref_rev_parse_rules is terminated with a NULL pointer.\nCompiling with \"gcc -O2 -Wall\" does not cause any problems, but\ncompiling with \"-O3 -Wall\" generates:\n\n  $ make CFLAGS='-O3 -Wall' refs.o\n  refs.c: In function ‘shorten_unambiguous_ref’:\n  refs.c:3379:29: warning: array subscript is above array bounds [-Warray-bounds]\n     for (; ref_rev_parse_rules[nr_rules]; nr_rules++)\n\nCuriously, we can silence this by explicitly nr_rules to 0\nin the beginning of the loop, even though the compiler\nshould be able to tell that we follow this code path only\nwhen nr_rules is already 0.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI've convinced myself that this is a gcc bug and not some weird\nundefined behavior or extra analysis that gcc can do due to inlined\nfunctions. The fact that what should be a noop makes the warning go away\nmakes me very suspicious.\n\nYou can also silence it by declaring ref_rev_parse_rules as:\n\n  const char * const ref_rev_parse_rules[];\n\nto make both the strings themselves and the pointers in the list\nconstant. And that may be worth doing instead, because it really is\na constant list for us. The downside is that it's a little uglier to\nread, and it carries over to pointers we use to access it, like:\n\n  const char * const *p;\n  for (p = ref_rev_parse_rules; *p; p++)\n     ...\n\n refs.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex ad5d66c..c1cc98a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3376,7 +3376,7 @@ char *shorten_unambiguous_ref(const char *refname, int strict)\n \t\tsize_t total_len = 0;\n \n \t\t/* the rule list is NULL terminated, count them first */\n-\t\tfor (; ref_rev_parse_rules[nr_rules]; nr_rules++)\n+\t\tfor (nr_rules = 0; ref_rev_parse_rules[nr_rules]; nr_rules++)\n \t\t\t/* no +1 because strlen(\"%s\") < strlen(\"%.*s\") */\n \t\t\ttotal_len += strlen(ref_rev_parse_rules[nr_rules]);\n \n-- \n1.8.4.1.4.gf327177\n"}]}