Re: [PATCH 2/2] Use xmemdup in many places.

[Junio C Hamano <gitster@pobox.com>]

Pierre Habouzit <madcoder@debian.org> writes:

> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
>  attr.c                  |    7 +------
>  builtin-add.c           |    8 ++------
>  builtin-apply.c         |   11 ++---------
>  builtin-fetch--tool.c   |    6 +-----
>  builtin-fmt-merge-msg.c |   17 ++++++-----------
>  builtin-for-each-ref.c  |   40 +++++++++-------------------------------
>  builtin-log.c           |   12 ++----------
>  builtin-ls-files.c      |    9 +--------
>  builtin-mv.c            |    5 +----
>  builtin-revert.c        |    4 +---
>  builtin-shortlog.c      |   11 ++---------
>  commit.c                |   16 ++++++----------
>  connect.c               |    4 +---
>  convert.c               |    7 +------
>  diff.c                  |   13 ++-----------
>  diffcore-order.c        |    7 ++-----
>  fast-import.c           |    4 +---
>  http-push.c             |    9 ++-------
>  imap-send.c             |   20 +++++---------------
>  merge-recursive.c       |   19 ++++---------------
>  refs.c                  |   12 ++++--------
>  sha1_file.c             |   12 +++---------
>  tag.c                   |    4 +---
>  23 files changed, 60 insertions(+), 197 deletions(-)
> ...
> diff --git a/builtin-apply.c b/builtin-apply.c
> index 05011bb..900d0a7 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -293,11 +293,7 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)
>  			return def;
>  	}
>  
> -	name = xmalloc(len + 1);
> -	memcpy(name, start, len);
> -	name[len] = 0;
> -	free(def);
> -	return name;
> +	return xmemdup(start, len);
>  }

Did we start leaking "def" here? 

> diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
> index 0afa1c5..287d52a 100644
> --- a/builtin-for-each-ref.c
> +++ b/builtin-for-each-ref.c
> ...
> @@ -305,46 +301,28 @@ static const char *find_wholine(const char *who, int wholen, const char *buf, un
> ...
>  static const char *copy_name(const char *buf)
>  {
> -	const char *eol = strchr(buf, '\n');
> -	const char *eoname = strstr(buf, " <");
> -	char *line;
> -	int len;
> -	if (!(eoname && eol && eoname < eol))
> -		return "";
> -	len = eoname - buf;
> -	line = xmalloc(len + 1);
> -	memcpy(line, buf, len);
> -	line[len] = 0;
> -	return line;
> +	const char *cp;
> +	for (cp = buf; *cp != '\n'; cp++) {
> +		if (!strncmp(cp, " <", 2))
> +			return xmemdup(buf, cp - buf);
> +	}
> +	return "";
>  }

At least the loop should terminate upon (!*cp); if you do not
have '\n' in the buffer what happens?