From: Junio C Hamano Date: Wed, 16 Sep 2015 18:43:49 GMT Subject: Re: [PATCH 23/67] add_packed_git: convert strcpy into xsnprintf Message-ID: In-Reply-To: <20150915154143.GW29753@sigill.intra.peff.net> Jeff King 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. > + > + 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;