Re: [PATCH 5/7] http: init and cleanup separately from http-walker

From: Tay Ray Chuan <rctay89@gmail.com>
Date: Feb 21, 2010, 15:23 UTC
Message-ID: <be6fef0d1002210723j742c0d8ex86b9d552f924408f@mail.gmail.com>
In-Reply-To: <20100221105707.GA22534@localhost>

Hi,

On Sun, Feb 21, 2010 at 6:57 PM, Clemens Buchacher <drizzd@aon.at> wrote:

Show 23 quoted lines

> 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