Fri 05 Apr 2013 07:42:13 AM UTC, comment #3:
Thank you for your response.
> wouldn't that leak the socket if it is not valid?
I scanned wget-1.14 without the patch and with the patch
using static analysis tool Coverity 6.5.1. It didn't find
any added errors, so there should be NO resource leak. I know
static analysis is not perfect, but from my experiences with
Coverity I must say it is very reliable.
Anyway there is the following construction:
if (persistent_available_p(...))
{
...
}
else if (host_lookup_failed)
{
...
}
else if (sock != -1)
{
sock = -1;
}
persistent_available_p(...) returns false only if there is NO active
connection OR if we want to use new connection (SSL) or if there
was some problem with connection and it was invalidated. In other
words if persistent_available_p(...) doesn't returned true, then we
have to create new connection anyway.
> Should you perhaps use CLOSE_INVALIDATE?
Problem is that persistent_available_p(...) deals with "pconn.socket"
while outside of this function we use "sock" variable. Even if we used
CLOSE_INVALIDATE() in persistent_available_p() we have to set "sock"
to -1 because we might be using already closed socket. Also calling
CLOSE_INVALIDATE() on "sock" if persistent_available_p() returned
false would be not a good idea, because we might be closing already
closed socket.
> Can you also add the entry to the src/ChangeLog file?
I attached new patch with entry in src/ChangeLog.
(file #27778)
|