Jeff King <p...@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.

> +
> +     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;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to