Re: [bug] blame duplicates trailing ">" in mailmapped emails

From: Jeff King <peff@peff.net>
Date: Feb 5, 2012, 23:47 UTC
Message-ID: <20120205234750.GA28735@sigill.intra.peff.net>
In-Reply-To: <7vipjlezas.fsf@alter.siamese.dyndns.org>

On Sun, Feb 05, 2012 at 01:38:19PM -0800, Junio C Hamano wrote:

Show 10 quoted lines

> > The map_user() API takes an email address that is terminated by either NUL
> > or '>' to allow the caller to learn the corresponding up-to-date email
> > address that is NUL terminated, while indicating with its return value
> > that if the caller even needs to replace what it already has.  But the
> > function does not properly terminate email when it only touched the name
> > part. And I think that is the real bug.
> 
> And the gist of the patch to fix the bug would look like this two liner.
> In the real fix, "p" should be renamed to "end_of_email" or something
> descriptive like that.

Exactly; this is much better.

We could also go as far as saying that map_user would _always_ terminate in this way (i.e., the caller gets a munged result, whether we found anything or not). Then internally, map_user could be simplified to stop worrying about making a temporary copy in mailbuf. And callers could simply call map_user without worrying about branching on whether it found anything or not.

But maybe it is not worth that level of refactoring. From my reading, your patch fixes the problem just fine.

-Peff