Re: [PATCH] split_ident: parse timestamp from end of line

On Mon, Oct 14, 2013 at 03:25:29PM -0700, Junio C Hamano wrote:

quoted

+	/*
+	 * Look from the end-of-line to find the trailing ">" of the mail
+	 * address, even though we should already know it as split->mail_end.
+	 * This can help in cases of broken idents with an extra ">" somewhere
+	 * in the email address.  Note that we are assuming the timestamp will
+	 * never have a ">" in it.
+	 *
+	 * Note also that this memchr can never return NULL, as we would
+	 * always find at least the split->mail_end closing bracket.
+	 */
+	cp = memrchr(split->mail_end, '>', len - (split->mail_end - line));
+	for (cp = cp + 1; cp < line + len && isspace(*cp); cp++)
 		;

"git grep" tells me this is the first use of memrchr(), which,
unlike memchr(), is _GNU_SOURCE-only if I am not mistaken, so we may
need a fallback definition in the compat/ and NEEDS_MEMRCHR in the
Makefile, I think.

Yeah, you are right[1]. I'm happy to re-roll. I wonder if we even need
to worry about a compatibility wrapper. We are already doing pointer
manipulations, and it is probably just as readable to roll the loop by
hand.

-Peff

[1] I even looked at "man memrchr" on my glibc system and was surprised
    to see it mentioned above the "#define _GNU_SOURCE" fold. But that
    "fold" is used only sometimes (e.g., strchrnul), and not others (in
    memrchr, the portability bits are listed at the end of the
    synopsis). Grr.