Re: [PATCH] Remove possible segfault in http-fetch.
- From
Junio C Hamano <junkio@cox.net>
- Date
- May 21, 2006, 07:49 UTC
- Message-ID
- <7vverzzukg.fsf@assigned-by-dhcp.cox.net>
- In-Reply-To
- <BAYC1-PASMTP082397700A9527CC2F3786AEA40@CEZ.ICE>
Sean <seanlkml@sympatico.ca> writes:
Show 29 quoted lines
> Free the curl string lists after running http_cleanup to
> avoid an occasional segfault in the curl library. Seems
> to only occur if the website returns a 405 error.
>...
> It comes with a big disclaimer because I don't really know the
> code in here all that well. However gdb reports the segfault
> happens in a strncasecmp call, and seeing as we've released a
> bunch of strings prior to the call....
>
> http-fetch.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/http-fetch.c b/http-fetch.c
> index 861644b..178f1ee 100644
> --- a/http-fetch.c
> +++ b/http-fetch.c
> @@ -1269,10 +1269,10 @@ int main(int argc, char **argv)
> if (pull(commit_id))
> rc = 1;
>
> - curl_slist_free_all(no_pragma_header);
> -
> http_cleanup();
>
> + curl_slist_free_all(no_pragma_header);
> +
> if (corrupt_object_found) {
> fprintf(stderr,
> "Some loose object were found to be corrupt, but they might be just\n"curl_easy_cleanup() which is called from http_cleanup() says it is safe to remove the strings _after_ you call that function, so I think the change makes sense -- it was apparently unsafe to free them before calling cleanup.
Knowing nothing about quirks in curl libraries, one thing that is mystery to me is that we slist_append() to other two lists (pragma_header and range_header) but we do not seem to ever free them. Another slist dav_headers is allocated and then freed inside a function, so that call-pattern seems well-formed.
Nick, care to help us out?