git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Felipe Contreras" <felipe.contreras@gmail.com>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	git@vger.kernel.org, "SZEDER Gábor" <szeder@ira.uka.de>
Subject: Re: [bug] blame duplicates trailing ">" in mailmapped emails
Date: Sun, 5 Feb 2012 18:47:50 -0500	[thread overview]
Message-ID: <20120205234750.GA28735@sigill.intra.peff.net> (raw)
In-Reply-To: <7vipjlezas.fsf@alter.siamese.dyndns.org>

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

> > 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

  reply	other threads:[~2012-02-05 23:48 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-02  1:15 [PATCH v3 0/4] completion: couple of cleanups Felipe Contreras
     [not found] ` <1328145320-14071-2-git-send-email-felipe.contreras@gmail.com>
2012-02-02  8:16   ` [PATCH v3 1/4] completion: be nicer with zsh Jonathan Nieder
2012-02-02  8:34     ` Felipe Contreras
2012-02-02  9:10       ` Jonathan Nieder
2012-02-02  9:38         ` Felipe Contreras
2012-02-02  9:46           ` Jonathan Nieder
2012-02-02 10:18             ` Felipe Contreras
2012-02-02  8:48 ` Jonathan Nieder
2012-02-02 10:12   ` Felipe Contreras
2012-02-02 10:35     ` Thomas Rast
2012-02-02 10:50       ` Jonathan Nieder
2012-02-02 10:55         ` Jonathan Nieder
2012-02-02 11:00       ` Felipe Contreras
2012-02-02 19:27   ` Junio C Hamano
2012-02-02 20:45     ` Felipe Contreras
2012-02-03  0:17       ` Junio C Hamano
2012-02-03 10:38         ` Felipe Contreras
2012-02-03 20:28           ` Junio C Hamano
2012-02-04 15:46             ` Felipe Contreras
2012-02-04 18:26               ` [bug] blame duplicates trailing ">" in mailmapped emails Jeff King
2012-02-04 19:30                 ` Felipe Contreras
2012-02-04 23:20                   ` Jeff King
2012-02-05 21:11                     ` Felipe Contreras
2012-02-05 23:50                       ` Jeff King
2012-02-05 20:16                 ` Junio C Hamano
2012-02-05 21:38                   ` Junio C Hamano
2012-02-05 23:47                     ` Jeff King [this message]
2012-02-06  0:39                       ` Junio C Hamano
2012-02-06  3:03                         ` Jeff King
2012-02-06  3:13                           ` Junio C Hamano
2012-02-06  3:16                           ` Junio C Hamano
2012-02-06  4:01                             ` Jeff King
2012-02-06 12:14                             ` Felipe Contreras
2012-02-06 22:04                               ` Junio C Hamano
2012-02-06 23:11                                 ` Felipe Contreras
2012-02-05 20:49               ` [PATCH v3 1/4] completion: be nicer with zsh Junio C Hamano
2012-02-02  9:05 ` [PATCH v3 2/4] completion: simplify __git_remotes Jonathan Nieder
2012-02-02 19:48 ` [PATCH v3 0/4] completion: couple of cleanups Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120205234750.GA28735@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=szeder@ira.uka.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).