Re: [PATCH v2] http*: cleanup slot->local after fclose

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 1, 2009, 16:13 UTC
Message-ID: <7vws7wkk4j.fsf@alter.siamese.dyndns.org>
In-Reply-To: <be6fef0d0906010652v4fc814f9j631795cbad61be9@mail.gmail.com>

Tay Ray Chuan <rctay89@gmail.com> writes:

Show 11 quoted lines

> the problem isn't triggered by fclose(slot->local) being done twice,
> but a ftell() being done on slot->local, which points to a FILE*
> handle that has already been fclose()'d (that is what the valgrind log
> provided by Clemens reported). The offending ftell() is found in
> http.c::run_active_slot(), and the code there does indeed depend on
> the NULLness of slot->local (it checks whether slot->local is NULL
> before doing the ftell()).
>
> run_active_slot() is used rather heavily, and users of slot->local
> always neglect to set it to NULL doing a fclose() on the FILE* handle
> it points to.

Thanks; that clearly indicates that you are fixing an existing bug.

I wish the commit log message explained that from the beginning without being asked.

So what would we want to do?

No patch from your http-push topic is in 'next' yet, and I presume that the same issue exists in 'maint', so I'd say:

 (0) queue your slimmed down fix to 'maint' (or rc/maint-http-local-slot-fix
     that is forked from 'maint', to later merge to 'maint');

 (1) create a new rc/http-push topic branch from 'master';

 (2) merge (0) into the result of (1);
 
 (3) re-apply the patches queued on the current rc/http-push topic on top
     of the result of (2).

to rebuild it, perhaps?