From: Tay Ray Chuan Date: Sun, 21 Feb 2010 15:23:30 GMT Subject: Re: [PATCH 5/7] http: init and cleanup separately from http-walker Message-ID: In-Reply-To: <20100221105707.GA22534@localhost> Hi, On Sun, Feb 21, 2010 at 6:57 PM, Clemens Buchacher wrote: > On Sun, Feb 21, 2010 at 11:38:20AM +0100, Clemens Buchacher wrote: >> On Sun, Feb 21, 2010 at 11:08:26AM +0800, Tay Ray Chuan wrote: >> > diff --git a/http-fetch.c b/http-fetch.c >> [...] >> >>         add_fill_function(walker, (int (*)(void *)) fill_active_slot); >> >> already deals with curl functionality. So even though I think it technically >> doesn't break, I would prefer if this dependency were still expressed in the >> code. >> >> > @@ -88,6 +90,7 @@ int main(int argc, const char **argv) >> >  "status code.  Suggest running 'git fsck'.\n"); >> >     } >> > >> > +   http_cleanup(); >> >     walker_free(walker); >> > >> >     free(rewritten_url); >> >> Same as above. > > But I think this is still valid. regarding the order of invocation - I could put http_cleanup() after walker_free(). Perhaps I can express this dependency with a comment in that area in http-fetch.c and/or http-walker::cleanup()? Don't invoke http_cleanup() yet, we might still need to perform http operations in http-walker. (Separately, on re-reading the code, I noticed that http-walker::cleanup() calls http_cleanup() again, so I dropped that invocation from http-walker.) -- Cheers, Ray Chuan