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

From: Jeff King <peff@peff.net>
Date: Sep 16, 2015, 20:42 UTC
Message-ID: <20150916204226.GE3915@sigill.intra.peff.net>
In-Reply-To: <xmqqlhc6p2f9.fsf@gitster.mtv.corp.google.com>

On Wed, Sep 16, 2015 at 12:52:26PM -0700, Junio C Hamano wrote:

Show 12 quoted lines

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

Yeah, though that is nothing new.

In some of the cases, as you've seen, I dug further in cleaning things up. But in others I did the minimal fix (especially in this case, the limitations are only about the deprecated "branches" and "remotes" file), mostly to try to keep the scope of work sane.

Show 8 quoted lines

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

I'll take a look and see how painful this is.

-Peff