Re: [PATCH 33/67] read_branches_file: replace strcpy with xstrdup

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 16, 2015, 19:52 UTC
Message-ID: <xmqqlhc6p2f9.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20150915154950.GG29753@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 19 quoted lines

> This code is exactly replicating strdup, so let's just use
> that. It's shorter, and eliminates some confusion (such as
> whether "p - s" is really enough to hold the result; it is,
> because we write NULs as we shrink "p").
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  remote.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 5ab0f7f..1b69751 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -297,7 +297,6 @@ static void read_branches_file(struct remote *remote)
>  	int n = 1000;
>  	FILE *f = fopen(git_path("branches/%.*s", n, remote->name), "r");
>  	char *s, *p;
> -	int len;

Hmm, we would punish those with ridiculously long remote name by truncating at n but that is OK.

We use buffer[BUFSIZ] to read various things in this file, not just $GIT_DIR/branches/* files, with fgets(), which may be better done if we switched to strbuf_getline(). Then we can also use trim family of calls from the strbuf API suite.

Move to strbuf_getline() may be a doubly attractive proposition, with a possible change to strbuf_getline() to make it also remove CR that immediately precedes LF [*1*], helping DOSsy platforms.

[Reference]

*1* http://thread.gmane.org/gmane.comp.version-control.msysgit/21773/focus=21780

Show 14 quoted lines

>  
>  	if (!f)
>  		return;
> @@ -313,9 +312,7 @@ static void read_branches_file(struct remote *remote)
>  	p = s + strlen(s);
>  	while (isspace(p[-1]))
>  		*--p = 0;
> -	len = p - s;
> -	p = xmalloc(len + 1);
> -	strcpy(p, s);
> +	p = xstrdup(s);
>  
>  	/*
>  	 * The branches file would have URL and optionally