Jeff King <peff@xxxxxxxx> writes:
> Converting it to use strbuf looks like it will actually let us drop a
> bunch of copying, too, as we just end up in mkpath at the very lowest
> level. I.e., something like below.
Thanks; I may have a few minor comments, but overall, I like the
placement of mkpath() in the resulting callchain a lot better than
the original.
> As an aside, I have noticed us using this "push/pop" approach to treating a
> strbuf as a stack of paths elsewhere, too. I.e:
>
> size_t baselen = base->len;
> strbuf_addf(base, "/%s", some_thing);
> some_call(base);
> base->len = baselen;
>
> I wondered if there was any kind of helper we could add to make it look
> nicer. But I don't think so; the hairy part is that you must remember to
> reset base->len after the call, and there is no easy way around that in
> C. If you had object destructors that ran as the stack unwound, or
> dynamic scoping, it would be easy to manipulate the object. Wrapping
> won't work because strbuf isn't just a length wrapping an immutable
> buffer; it actually has to move the NUL in the buffer.
>
> Anyway, not important, but perhaps somebody is more clever than I am.
Hmph... interesting but we would need a lot more thought than the
time necessary to respond to one piece of e-mail for this ;-)
Perhaps later...
> diff --git a/builtin/prune.c b/builtin/prune.c
> index 6366917..4ca8ec1 100644
> --- a/builtin/prune.c
> +++ b/builtin/prune.c
> @@ -17,9 +17,8 @@ static int verbose;
> static unsigned long expire;
> static int show_progress = -1;
>
> -static int prune_tmp_object(const char *path, const char *filename)
> +static int prune_tmp_object(const char *fullpath)
> {
> - const char *fullpath = mkpath("%s/%s", path, filename);
> struct stat st;
> if (lstat(fullpath, &st))
> return error("Could not stat '%s'", fullpath);
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.
> -static int prune_dir(int i, char *path)
> +static int prune_dir(int i, struct strbuf *path)
> {
> - DIR *dir = opendir(path);
> + size_t baselen = path->len;
> + DIR *dir = opendir(path->buf);
> struct dirent *de;
>
> if (!dir)
> @@ -77,28 +76,39 @@ static int prune_dir(int i, char *path)
> if (lookup_object(sha1))
> continue;
>
> - 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.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html