Re: [PATCH 23/67] add_packed_git: convert strcpy into xsnprintf
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 16, 2015, 18:43 UTC
- Message-ID
- <xmqq8u86qk62.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20150915154143.GW29753@sigill.intra.peff.net>
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.
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?
If ".bitmap" were in the set of suffixes we care about (it isn't), we would be using that instead when measuring "alloc".
Thanks.
Show 10 quoted lines
> +
> + xsnprintf(p->pack_name + path_len, alloc - path_len, ".keep");
> if (!access(p->pack_name, F_OK))
> p->pack_keep = 1;
>
> - strcpy(p->pack_name + path_len, ".pack");
> + xsnprintf(p->pack_name + path_len, alloc - path_len, ".pack");
> if (stat(p->pack_name, &st) || !S_ISREG(st.st_mode)) {
> free(p);
> return NULL;