Re: [PATCH 23/67] add_packed_git: convert strcpy into xsnprintf

From: Jeff King <peff@peff.net>
Date: Sep 16, 2015, 20:24 UTC
Message-ID: <20150916202400.GB3915@sigill.intra.peff.net>
In-Reply-To: <xmqq8u86qk62.fsf@gitster.mtv.corp.google.com>

On Wed, Sep 16, 2015 at 11:43:49AM -0700, Junio C Hamano wrote:

Show 9 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > +	alloc = path_len + strlen(".pack") + 1;
> > +	p = alloc_packed_git(alloc);
> > +	memcpy(p->pack_name, path, path_len); /* NUL from zero-ed struct */
> 
> This comment is confusing, isn't it?  Yes, there is a NUL, but you
> will going to overwrite it with "." in ".keep" immediately and more
> importantly, that overwriting does not depend on NUL being there.

Yeah, you're right. I was blindly making sure that the behavior did not change from the original, without noticing that the original did not care about the NUL either way.

Show 5 quoted lines

> What's more important to comment on would probably be the line that
> computes the "alloc".  It uses ".pack" but that is because it knows
> that is the longest suffix we care about, and that deserves mention
> more than the NUL termination of intermediate result that does not
> matter, no?

Agreed. I'll add a comment to that effect.

-Peff