Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer

On Wed, Dec 18, 2013 at 11:35:34AM -0800, Junio C Hamano wrote:

This function is called to remove

 * Any tmp_* found directly in .git/objects/
 * Any tmp_* found directly in .git/objects/pack/
 * Any tmp_obj_* found directly in .git/objects/??/

and shares the same expiration logic with prune_object().  The only
difference from the other function is what the file is called in
dry-run or verbose report ("stale temporary file" vs "<sha-1> <typename>").

We may want to rename it to prune_tmp_file(); its usage may have
been limited to an unborn loose object file at some point in the
history, but it does not look that way in today's code.

Yes, certainly the rename makes sense (and I think the history is as you
mentioned). I noticed the similarity between the two, as well. It might
be simple to refactor into a single function.

quoted

-			prune_object(path, de->d_name, sha1);
+			strbuf_addf(path, "/%s", de->d_name);
+			prune_object(path->buf, sha1);
+			path->len = baselen;

This is minor, but I prefer using strbuf_setlen() for this.

Good catch. I do not think it is minor at all; my version is buggy.
After the loop ends, path->len does not match the NUL in path->buf. That
is OK if the next caller is strbuf-aware, but if it were to pass
path->buf straight to a system call, that would be rather...confusing.

-Peff