Re: [PATCH v15 07/13] ref-filter: add support for %(contents:lines=X)

Eric Sunshine [off-list ref] writes:

quoted

@@ -624,6 +633,33 @@ static void find_subpos(const char *buf, unsigned long sz,
        *nonsiglen = *sig - buf;
 }

+/*
+ * If 'lines' is greater than 0, append that many lines from the given
+ * 'buf' of length 'size' to the given strbuf.
+ */
+static void append_lines(struct strbuf *out, const char *buf, unsigned long size, int lines)
+{
+       int i;
+       const char *sp, *eol;
+       size_t len;
+
+       if ((sp = strstr(buf, "\n\n")) && (sp <= buf + size))
+               size += 2;

Aside from the +2 which Matthieu already questioned, this code has a
much more serious problem. strstr() assumes that 'buf' is
NUL-terminated, however, the fact that buf's size is also being passed
to the function, implies that it may not be NUL-terminated.

If Karthik applies my suggestion, then the strstr would go away. I think
the code would be correct even on non-null-terminated strings.

Actually, we're already making the assumption that the buffer for the
whole tag object is null-terminated (and contains no '\0') for
%(contents):

		else if (!strcmp(name, "contents"))
			v->s = xstrdup(subpos);

(But I agree that even if the assumption is correct, it should be made
explicit if it remains a precondition of append_lines).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/