cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [ 934666 ] storage leak in ftp.c

From: Daniel Stenberg <daniel-curl_at_haxx.se>
Date: Thu, 15 Apr 2004 08:42:36 +0200 (CEST)

Hi

(Regarding bug report #934666, details found here:
http://sourceforge.net/tracker/index.php?func=detail&aid=934666&group_id=976&atid=100976)

I don't like having long conversations in the bug tracker, I prefer using my
trusty old email client for that!

Ok, to put it short:

1. We have 40+ test cases in the test suite, a bunch of them test fail cases.
   None of them leak memory. I even added a test case yesterday (511) to use
   libcurl in a fashion more similar to your described problem and it shows
   no leak either. We have a built-in leak detector when curl is built with
   debug.

2. I'll reply to a paragraph in the most recent comment in the report:

> But you can also examine the code and see that this is a problem, even
> without a test case. In the Curl_ftp function, both ftp->dirs[] and
> ftp->file are set to allocated strings. In the error handling at the very
> end of the function, the strings in ftp->dirs[] are freed, but not
> ftp->file.

Right. But you need to consider the whole function flow for it to be a leak.
I'll tell you why I don't see this lack of code as a leak:

In libcurl, we use functions that basicly are DO and DONE (named Curl_do()
and Curl_done()). For each file we want to transfer, the DO function is called
and when it is complete the DONE function is called. The DONE function is
called unconditionally, even if the DO function failed. In these functions,
protocol-specific sub-functions might get called, which in the FTP case are
called Curl_ftp() (for DO) and Curl_ftp_done() (for DONE).

This allows Curl_ftp_done() to be used to cleanup whatever was allocated for a
single particular FTP transfer.

So, if this function is called after a transfer (after a DO really) then
ftp->file is freed in there. If this function is not called after DO has been
called, then we have a leak and then we are doing wrong in more aspects than
just that single leak.

I'm not saying you're wrong, I'm saying that I don't see this problem and I
can't repeat it and thus I hesitate to change the code...

Some info on my current development: I'm moving the free(ftp->file) call to
the freedirs() function since they should all be freed at once anyway. I'm
also currently redoing how the ftp->dirs[] array is setup as I am removing the
fixed 100 dir depth limit libcurl has had until now.

-- 
     Daniel Stenberg -- http://curl.haxx.se -- http://daniel.haxx.se
      Dedicated custom curl help for hire: http://haxx.se/curl.html
Received on 2004-04-15