[RFH] lifetime rule for url parameter to transport_get()?

Does anybody remember why we use a copied string of "ref_git_copy" in builtin/clone.c::setup_reference()?

	ref_git = real_path(option_reference);
	...
	ref_git_copy = xstrdup(ref_git);
	add_to_alternates_file(ref_git_copy);
	remote = remote_get(ref_git_copy);
	transport = transport_get(remote, ref_git_copy);
	for (extra = transport_get_remote_refs(transport); extra;
	     extra = extra->next)
		add_extra_ref(extra->name, extra->old_sha1, 0);
	transport_disconnect(transport);
	free(ref_git_copy);

The three functions add_to_alternates_file(), remote_get(), and transport_get() all get "const char *" so I do not think the copy was done to avoid "option_reference" from getting clobbered by these functions. The only thing I can think of is that transport_get() does this:

    struct transport *transport_get(struct remote *remote, const char *url)
    {
            const char *helper;
            struct transport *ret = xcalloc(1, sizeof(*ret));

            ...
            if (!url && remote->url)
                    url = remote->url[0];
            ret->url = url;
	    ...
	    return ret;
    }

holding onto "url" without making a copy for its own use. But then freeing that copy by the caller after calling transport_disconnect() does not make much sense to me---we could have just gave it the original option_reference, have transport use it while it runs ls-remote equivalent, and then called transport_disconnect(), without using any extra copy.

What I am missing?

Junio C Hamano <gitster@pobox.com> writes:

It didn't have anything to do with transport/remote layer.

This codepath uses real_path() and optionally mkpath(), both of which returns a short-lived static buffer to return its findings, and long-term users are expected to copy it away.

I'll add a comment to that effect in the code.

On Tue, 23 Aug 2011, Junio C Hamano wrote:

Yeah, that fits with my expectation, given the lack of a comment and the fact that you were asking about clone and not also fetch.

At least originally, the remote and transport data was expected to live until the process exits, since it's a small, bounded number of small objects. If I'd included functions to free the structures, I'd have had them free the strings they were passed.

	-Daniel
*This .sig left intentionally blank*