On Mon, 13 Jun 2011, Aaron Orenstein wrote:

>> Scratch that. I instead suggest the following patch, can you check if this
>> fixes the problem you experience?
>
> Yes - that fixes it - although it seems a bit heavy-handed to me. I prefer
> the way it was where the function that opened the file was the one that (was
> supposed to) close it as well.

I agree about that as a general principle, but I was mostly interested in
making sure I understood and covered your error case. I still don't quite
underst during which situation readfromfile() never gets a zero return code
from fread() and thus closes the file.

> I still think the "nread > size" is incorrect - under what circumstance
> (other than a misbehaving OS) would nread be larger than the requested read?

Look at the code again and see how it *might* call the read callback instead
of fread() and the user provided callback can indeed return a bigger value.

> On the other hand a short read ("nread < size") is expected - and matches
> the given comment.

"man fread" says:

        Upon successful completion, fread() shall return the number of elements
        successfully read which is less than nitems only if a read error or
        end-of-file is encountered.

So for the fread() case, which I believe is what you're using, the condition
seems to be correct to me as it does indeed trap the final fread(). Doesn't
it?

The flaw that I can find is that it treats the read callback return code the
same way, and we haven't documented this behavior/restriction anywhere.
Possibly this is reason enough to always do fread/callback until a zero is
returned and not bail out on short reads.

> You can test the problem without a windows machine by using a debugger and
> breaking at the fclose() line and noting that with the given test case
> fclose() is never called. Stepping through the code should give you a good
> idea of what is going on.

I don't think it's that simple to repeat. If your error case would trigger the
problem that easily then why doesn't unit test 1308 leak a FILE *?
https://github.com/bagder/curl/blob/master/tests/unit/unit1308.c

-- 
  / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html