On Sun, Mar 22, 2015 at 12:36 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> The cleanup function is used in 4 places now and it's always safe to
>> free up the memory as well.
>>
>> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
>> ---
>> http.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/http.c b/http.c
>> index 9c825af..4b179f6 100644
>> --- a/http.c
>> +++ b/http.c
>> @@ -1462,6 +1462,7 @@ void release_http_pack_request(struct http_pack_request *preq)
>> }
>> preq->slot = NULL;
>> free(preq->url);
>> + free(preq);
>> }
>>
>> int finish_http_pack_request(struct http_pack_request *preq)
>
> Freeing of preq in all the callers of this one looks sensible,
> except for the one in finish_request() of http-push.c that pulls an
> preq instance out of request->userData.
>
> Can somebody help me follow the dataflow to convince me that this is
> not leading to double-free in start_fetch_packed()?
I am not sure where you suspect the double free problem.
In start_fetch_packed we have a call to release_http_pack_request
2 times but just in an error-out-and-cleanup case, so either of one cases
is called.
In the latter place (http-push.c lines 335-347), we have code like
request->userData = preq;
if (!start_active_slot(preq->slot)) {
release_http_pack_request(preq);
repo->can_update_info_refs = 0;
release_request(request);
}
Do you mean that the release_http_pack_request and release_request may collide
as the `release_request(request);` has a pointer to preq via its userData field?
Well there is hope, as `release_request` only touches
free(request->url);
free(request);
and not the userData pointer.
I am a bit puzzled what you're trying to hint at.
>
> Thanks.
>
--
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