[3/4] xdiff: avoid computing non-zero offset from NULL pointer
diff mbox series

Message ID 20200125053929.GC744673@coredump.intra.peff.net
State New
Headers show
Series
  • more clang/sanitizer fixes
Related show

Commit Message

Jeff King Jan. 25, 2020, 5:39 a.m. UTC
As with the previous commit, clang-11's UBSan complains about computing
offsets from a NULL pointer, causing some tests to fail. In this case,
though, we're actually computing a non-zero offset, which is even more
dubious. From t7810:

  xdiff-interface.c:268:14: runtime error: applying non-zero offset 1 to null pointer
  ...
  not ok 131 - grep -p with userdiff

The problem is our parsing of the funcname config. We count the number
of lines in the string, allocate an array, and then loop over our
allocated entries, parsing each line and moving our cursor to one past
the trailing newline for the next iteration.

But the final line will not generally have a trailing newline (since
it's a config value), and hence we go to one past NULL. In practice this
is OK, since our loop should terminate before we look at the value. But
even computing such an invalid pointer technically violates the
standard.

We can fix it by leaving the pointer at NULL if we're at the end, rather
than one-past. And while we're thinking about it, we can also document
the variant by asserting that our initial line-count matches the
second-pass of parsing.

Signed-off-by: Jeff King <peff@peff.net>
---
 xdiff-interface.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Patch
diff mbox series

diff --git a/xdiff-interface.c b/xdiff-interface.c
index 2f1fe48512..3cd2ac2855 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -250,9 +250,13 @@  void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value, int cflags)
 	ALLOC_ARRAY(regs->array, regs->nr);
 	for (i = 0; i < regs->nr; i++) {
 		struct ff_reg *reg = regs->array + i;
-		const char *ep = strchr(value, '\n'), *expression;
+		const char *ep, *expression;
 		char *buffer = NULL;
 
+		if (!value)
+			BUG("mismatch between line count and parsing");
+		ep = strchr(value, '\n');
+
 		reg->negate = (*value == '!');
 		if (reg->negate && i == regs->nr - 1)
 			die("Last expression must not be negated: %s", value);
@@ -265,7 +269,7 @@  void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value, int cflags)
 		if (regcomp(&reg->re, expression, cflags))
 			die("Invalid regexp to look for hunk header: %s", expression);
 		free(buffer);
-		value = ep + 1;
+		value = ep ? ep + 1 : NULL;
 	}
 }