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

From: Jeff King <peff@peff.net>
Date: Oct 14, 2013, 22:31 UTC
Message-ID: <20131014223137.GA12744@sigill.intra.peff.net>
In-Reply-To: <xmqqwqlfebhi.fsf@gitster.dls.corp.google.com>

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

Show 18 quoted lines

> > +	/*
> > +	 * 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.