Re: [PATCH 4/5] pretty: Use mailmap to display username and email

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Antoine Pelisse <apelisse@xxxxxxxxx> writes:

> Use the mailmap information to display the correct
> username and email address in all log commands.
>
> Signed-off-by: Antoine Pelisse <apelisse@xxxxxxxxx>
> ---
>  pretty.c | 46 +++++++++++++++++++++++++++++++---------------
>  1 file changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/pretty.c b/pretty.c
> index 6730add..e232aaa 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -387,6 +387,8 @@ void pp_user_info(const struct pretty_print_context *pp,
>  		  const char *what, struct strbuf *sb,
>  		  const char *line, const char *encoding)
>  {
> +	char person_name[1024];
> +	char person_mail[1024];
>  	struct ident_split ident;
>  	int linelen, namelen;
>  	char *line_end, *date;
> @@ -405,41 +407,55 @@ void pp_user_info(const struct pretty_print_context *pp,
>  	if (split_ident_line(&ident, line, linelen))
>  		return;
>  
> -	namelen = ident.mail_end - ident.name_begin + 1;
> +	memcpy(person_mail, ident.mail_begin, ident.mail_end - ident.mail_begin);
> +	person_mail[ident.mail_end - ident.mail_begin] = 0;
> +
> +	memcpy(person_name, ident.name_begin, ident.name_end - ident.name_begin);
> +	person_name[ident.name_end - ident.name_begin] = 0;
> +
> +	if (pp->mailmap)
> +		map_user(pp->mailmap, person_mail, sizeof(person_mail),
> +			 person_name, sizeof(person_name));

This is why I said that we may want to rethink the API signature of
the map_user() function.  Now this caller has the hardcoded limit
for name and mail parts, and it needs to make copies of strings into
two arrays only because map_user() expects to get two fixed-size
buffers that are to be rewritten in-place.  If we changed map_user()
to take two strbufs (one for name, the other for mail) to be updated
in place, we would still need to make copies, but later code in this
function may be able to lose a few strlen() calls.

Or it might be better to make those two strbufs output-only
parameter, e.g.

	map_user(struct string_list *mailmap,
        	const char *name, size_t namelen,
                const char *mail, size_t maillen,
                struct strbuf *name_out, struct strbuf *mail_out);

then after split_ident_line(), this caller could feed two pointers
into the original "line" as name and mail parameter, without making
any copies (the callee has to make a copy but it has to be done
anyway when the name/mail is mapped).  I suspect it would make this
caller simpler, but I do not know how invasive such changes are for
other callers of map_user().

Such an update can be left outside of this series, of course.

> +		strbuf_addch(sb, ' ');
> +		strbuf_addch(sb, '<');
> +		strbuf_add(sb, person_mail, strlen(person_mail));
> +		strbuf_addch(sb, '>');
>  		strbuf_addch(sb, '\n');

Is that strbuf_addf(sb, " <%s>\n", person_mail)?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]