Stefan Beller <sbel...@google.com> writes:
> + struct strbuf sb = STRBUF_INIT;
> + char *ref_git = compute_alternate_path(item->string, &sb);
Who owns the memory for ref_git?
> - if (!access(mkpath("%s/shallow", ref_git), F_OK))
> - die(_("reference repository '%s' is shallow"), item->string);
> + if (!ref_git)
> + die("%s", sb.buf);
Presumably the second argument to compute_alternate_path() is a
strbuf to receive the error message? It is unfortunate that the
variable used for this purpose is a bland "sb", but perhaps that
cannot be helped as you would reuse that strbuf for a different
purpose (i.e. not to store the error message, but to formulate a
pathname).
> - if (!access(mkpath("%s/info/grafts", ref_git), F_OK))
> - die(_("reference repository '%s' is grafted"), item->string);
> + strbuf_addf(&sb, "%s/objects", ref_git);
> + add_to_alternates_file(sb.buf);
>
> - strbuf_addf(&alternate, "%s/objects", ref_git);
> - add_to_alternates_file(alternate.buf);
> - strbuf_release(&alternate);
> - free(ref_git);
> + strbuf_release(&sb);
I am wondering about the loss of free() here in the first comment.
> +/*
> + * Compute the exact path an alternate is at and returns it. In case of
> + * error NULL is returned and the human readable error is added to `err`
> + * `path` may be relative and should point to $GITDIR.
> + * `err` must not be null.
> + */
> +char *compute_alternate_path(const char *path, struct strbuf *err)
> +{
> + char *ref_git = NULL;
> + const char *repo, *ref_git_s;
> + struct strbuf err_buf = STRBUF_INIT;
Why do you need "err_buf", instead of directly writing the error to
"err", especially if "err" is not optional?
> + ...
> +out:
> + if (err_buf.len) {
> + strbuf_addbuf(err, &err_buf);
> + free(ref_git);
> + ref_git = NULL;
> + }
> +
> + strbuf_release(&err_buf);
> + return ref_git;
> +}
So ref_git is a piece of memory on heap, and the caller is
responsible for not leaking it.
--
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
