On Wed, 11 Feb 2009, Jay Soffian wrote:
> On Wed, Feb 11, 2009 at 7:18 PM, Jeff King <peff@xxxxxxxx> wrote:
> > On Wed, Feb 11, 2009 at 01:01:21AM -0500, Jay Soffian wrote:
> >
> >> +static void free_remote_ref_states(struct ref_states *states)
> >> +{
> >> + /* NEEDSWORK: free remote */
> >> + string_list_clear(&states->new, 0);
> >> + string_list_clear(&states->stale, 0);
> >> + string_list_clear(&states->tracked, 0);
> >> +}
> >
> > Hmm. I don't know anything about this code, so maybe it is not trivial.
> > But anytime you are touching an area that NEEDSWORK, I think it is worth
> > looking at whether you can fix that problem (since you have already
> > spent a few brain cycles understanding what is going on in general).
>
> I spent about 5 minutes which was enough time for me to realize that
> the reason the previous author left it as "NEEDSWORK" is because
> fixing it is more than 5 minutes of work. This is the remote object --
> maybe you could offer me some clues that allow me to know which of its
> fields need to be freed individually:
>
> struct remote {
> const char *name;
> int origin;
>
> const char **url;
> int url_nr;
> int url_alloc;
>
> const char **push_refspec;
> struct refspec *push;
> int push_refspec_nr;
> int push_refspec_alloc;
>
> const char **fetch_refspec;
> struct refspec *fetch;
> int fetch_refspec_nr;
> int fetch_refspec_alloc;
>
> /*
> * -1 to never fetch tags
> * 0 to auto-follow tags on heuristic (default)
> * 1 to always auto-follow tags
> * 2 to always fetch tags
> */
> int fetch_tags;
> int skip_default_update;
> int mirror;
>
> const char *receivepack;
> const char *uploadpack;
>
> /*
> * for curl remotes only
> */
> char *http_proxy;
> };
>
> I *think* const is a clue that the field need not be freed, because
> the pointer is to storage that is on the stack. But I wasn't sure, esp
> with the double pointers. And I really wasn't sure about the struct
> pointers.
Actually, the comment is wrong; "remote" comes from remote_get(), which
returns things from a cache in remote.c; there could be a remote_put() to
let the code know that the caller is done with the object, but it wouldn't
presently do anything.
(The code actually reads the config files once, generating info for all of
the configured remotes, and just returns them, except that it will
generate a new object for unconfigured, individually requested URLs)
-Daniel
*This .sig left intentionally blank*
--
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
