Network Kit: Coverity scan review and fixes
CID 1108353, 1108335: memory leak. CID 610473: unused variable. CID 1108446, 1108433, 1108432, 1108419, 1108400, 991710, 991713, 991712, 610098, 610097, 610096, 610095: uninitialized field CID 1108421: unused field Change the ownership of the result for Url/HttpRequests. The request now owns its result and you either access it by reference while the request is live, or copy it to keep it after the request destruction. To help with that, get BUrlResult copy constructor and assignment operator to work. Performance issue: copying the BUrlResult also copies the underlying BMallocIO data. This should be shared between the BUrlResult objects to make the copy lighter. The case of BUrlSynchronousRequest is now particularly inefficient, with at least 2 copies needed to get at the result.
This commit is contained in:
@@ -23,7 +23,7 @@ class BAbstractSocket;
|
|||||||
class BHttpRequest : public BUrlRequest {
|
class BHttpRequest : public BUrlRequest {
|
||||||
public:
|
public:
|
||||||
BHttpRequest(const BUrl& url,
|
BHttpRequest(const BUrl& url,
|
||||||
BUrlResult& result, bool ssl = false,
|
bool ssl = false,
|
||||||
const char *protocolName = "HTTP",
|
const char *protocolName = "HTTP",
|
||||||
BUrlProtocolListener* listener = NULL,
|
BUrlProtocolListener* listener = NULL,
|
||||||
BUrlContext* context = NULL);
|
BUrlContext* context = NULL);
|
||||||
|
|||||||
@@ -118,8 +118,6 @@ private:
|
|||||||
mutable bool fAuthorityValid : 1;
|
mutable bool fAuthorityValid : 1;
|
||||||
mutable bool fUserInfoValid : 1;
|
mutable bool fUserInfoValid : 1;
|
||||||
|
|
||||||
bool fBasicUri : 1;
|
|
||||||
|
|
||||||
bool fHasProtocol : 1;
|
bool fHasProtocol : 1;
|
||||||
bool fHasUserName : 1;
|
bool fHasUserName : 1;
|
||||||
bool fHasPassword : 1;
|
bool fHasPassword : 1;
|
||||||
|
|||||||
@@ -18,7 +18,6 @@ public:
|
|||||||
BUrlRequest(const BUrl& url,
|
BUrlRequest(const BUrl& url,
|
||||||
BUrlProtocolListener* listener,
|
BUrlProtocolListener* listener,
|
||||||
BUrlContext* context,
|
BUrlContext* context,
|
||||||
BUrlResult& result,
|
|
||||||
const char* threadName,
|
const char* threadName,
|
||||||
const char* protocolName);
|
const char* protocolName);
|
||||||
virtual ~BUrlRequest();
|
virtual ~BUrlRequest();
|
||||||
@@ -37,7 +36,7 @@ public:
|
|||||||
|
|
||||||
// URL protocol parameters access
|
// URL protocol parameters access
|
||||||
const BUrl& Url() const;
|
const BUrl& Url() const;
|
||||||
BUrlResult& Result() const;
|
const BUrlResult& Result() const;
|
||||||
BUrlContext* Context() const;
|
BUrlContext* Context() const;
|
||||||
BUrlProtocolListener* Listener() const;
|
BUrlProtocolListener* Listener() const;
|
||||||
const BString& Protocol() const;
|
const BString& Protocol() const;
|
||||||
@@ -63,7 +62,7 @@ protected:
|
|||||||
|
|
||||||
protected:
|
protected:
|
||||||
BUrl fUrl;
|
BUrl fUrl;
|
||||||
BUrlResult& fResult;
|
BUrlResult fResult;
|
||||||
BUrlContext* fContext;
|
BUrlContext* fContext;
|
||||||
BUrlProtocolListener* fListener;
|
BUrlProtocolListener* fListener;
|
||||||
|
|
||||||
|
|||||||
@@ -704,6 +704,7 @@ BHttpForm::_GetMultipartHeader(const BHttpFormData* element) const
|
|||||||
|
|
||||||
|
|
||||||
BHttpForm::Iterator::Iterator(BHttpForm* form)
|
BHttpForm::Iterator::Iterator(BHttpForm* form)
|
||||||
|
: fElement(NULL)
|
||||||
{
|
{
|
||||||
fForm = form;
|
fForm = form;
|
||||||
fStdIterator = form->fFields.begin();
|
fStdIterator = form->fFields.begin();
|
||||||
|
|||||||
@@ -28,11 +28,10 @@ static const char* kHttpProtocolThreadStrStatus[
|
|||||||
};
|
};
|
||||||
|
|
||||||
|
|
||||||
BHttpRequest::BHttpRequest(const BUrl& url, BUrlResult& result, bool ssl,
|
BHttpRequest::BHttpRequest(const BUrl& url, bool ssl, const char* protocolName,
|
||||||
const char* protocolName, BUrlProtocolListener* listener,
|
BUrlProtocolListener* listener, BUrlContext* context)
|
||||||
BUrlContext* context)
|
|
||||||
:
|
:
|
||||||
BUrlRequest(url, listener, context, result, "BUrlProtocol.HTTP", protocolName),
|
BUrlRequest(url, listener, context, "BUrlProtocol.HTTP", protocolName),
|
||||||
fSocket(NULL),
|
fSocket(NULL),
|
||||||
fSSL(ssl),
|
fSSL(ssl),
|
||||||
fRequestMethod(B_HTTP_GET),
|
fRequestMethod(B_HTTP_GET),
|
||||||
|
|||||||
@@ -631,6 +631,8 @@ BNetworkCookie::operator!=(const BNetworkCookie& other)
|
|||||||
void
|
void
|
||||||
BNetworkCookie::_Reset()
|
BNetworkCookie::_Reset()
|
||||||
{
|
{
|
||||||
|
fInitStatus = false;
|
||||||
|
|
||||||
fName.Truncate(0);
|
fName.Truncate(0);
|
||||||
fValue.Truncate(0);
|
fValue.Truncate(0);
|
||||||
fDomain.Truncate(0);
|
fDomain.Truncate(0);
|
||||||
|
|||||||
@@ -59,7 +59,7 @@ BNetworkCookieJar::BNetworkCookieJar(BMessage* archive)
|
|||||||
if (heapCookie == NULL)
|
if (heapCookie == NULL)
|
||||||
break;
|
break;
|
||||||
|
|
||||||
if (!AddCookie(heapCookie)) {
|
if (AddCookie(heapCookie) != B_OK) {
|
||||||
delete heapCookie;
|
delete heapCookie;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
@@ -69,9 +69,7 @@ BNetworkCookieJar::BNetworkCookieJar(BMessage* archive)
|
|||||||
|
|
||||||
BNetworkCookieJar::~BNetworkCookieJar()
|
BNetworkCookieJar::~BNetworkCookieJar()
|
||||||
{
|
{
|
||||||
BNetworkCookie* cookiePtr;
|
for (Iterator it = GetIterator(); it.Next() != NULL;)
|
||||||
|
|
||||||
for (Iterator it = GetIterator(); (cookiePtr = it.Next()) != NULL;)
|
|
||||||
delete it.Remove();
|
delete it.Remove();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -52,6 +52,8 @@ BUrl::BUrl(BMessage* archive)
|
|||||||
|
|
||||||
if (archive->FindString(kArchivedUrl, &url) == B_OK)
|
if (archive->FindString(kArchivedUrl, &url) == B_OK)
|
||||||
SetUrlString(url);
|
SetUrlString(url);
|
||||||
|
else
|
||||||
|
_ResetFields();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@@ -139,6 +141,7 @@ BUrl::BUrl()
|
|||||||
fRequest(),
|
fRequest(),
|
||||||
fHasAuthority(false)
|
fHasAuthority(false)
|
||||||
{
|
{
|
||||||
|
_ResetFields();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -20,14 +20,12 @@
|
|||||||
BUrlProtocolRoster::MakeRequest(const BUrl& url,
|
BUrlProtocolRoster::MakeRequest(const BUrl& url,
|
||||||
BUrlProtocolListener* listener, BUrlContext* context)
|
BUrlProtocolListener* listener, BUrlContext* context)
|
||||||
{
|
{
|
||||||
BUrlResult* result = new BUrlResult(url);
|
|
||||||
// FIXME this is leaked
|
|
||||||
// TODO: instanciate the correct BUrlProtocol using add-on interface
|
// TODO: instanciate the correct BUrlProtocol using add-on interface
|
||||||
if (url.Protocol() == "http") {
|
if (url.Protocol() == "http") {
|
||||||
return new(std::nothrow) BHttpRequest(url, *result, false, "HTTP", listener,
|
return new(std::nothrow) BHttpRequest(url, false, "HTTP", listener,
|
||||||
context);
|
context);
|
||||||
} else if (url.Protocol() == "https") {
|
} else if (url.Protocol() == "https") {
|
||||||
return new(std::nothrow) BHttpRequest(url, *result, true, "HTTPS", listener,
|
return new(std::nothrow) BHttpRequest(url, true, "HTTPS", listener,
|
||||||
context);
|
context);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -28,15 +28,15 @@ static const char* kProtocolThreadStrStatus[B_PROT_THREAD_STATUS__END+1]
|
|||||||
|
|
||||||
|
|
||||||
BUrlRequest::BUrlRequest(const BUrl& url, BUrlProtocolListener* listener,
|
BUrlRequest::BUrlRequest(const BUrl& url, BUrlProtocolListener* listener,
|
||||||
BUrlContext* context, BUrlResult& result, const char* threadName,
|
BUrlContext* context, const char* threadName, const char* protocolName)
|
||||||
const char* protocolName)
|
|
||||||
:
|
:
|
||||||
fUrl(url),
|
fUrl(url),
|
||||||
fResult(result),
|
fResult(url),
|
||||||
fContext(context),
|
fContext(context),
|
||||||
fListener(listener),
|
fListener(listener),
|
||||||
fQuit(false),
|
fQuit(false),
|
||||||
fRunning(false),
|
fRunning(false),
|
||||||
|
fThreadStatus(B_PROT_PAUSED),
|
||||||
fThreadId(0),
|
fThreadId(0),
|
||||||
fThreadName(threadName),
|
fThreadName(threadName),
|
||||||
fProtocol(protocolName)
|
fProtocol(protocolName)
|
||||||
@@ -46,6 +46,7 @@ BUrlRequest::BUrlRequest(const BUrl& url, BUrlProtocolListener* listener,
|
|||||||
|
|
||||||
BUrlRequest::~BUrlRequest()
|
BUrlRequest::~BUrlRequest()
|
||||||
{
|
{
|
||||||
|
Stop();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@@ -168,7 +169,7 @@ BUrlRequest::Url() const
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
BUrlResult&
|
const BUrlResult&
|
||||||
BUrlRequest::Result() const
|
BUrlRequest::Result() const
|
||||||
{
|
{
|
||||||
return fResult;
|
return fResult;
|
||||||
|
|||||||
@@ -18,7 +18,8 @@ BUrlResult::BUrlResult(const BUrl& url)
|
|||||||
:
|
:
|
||||||
fUrl(url),
|
fUrl(url),
|
||||||
fRawData(),
|
fRawData(),
|
||||||
fHeaders()
|
fHeaders(),
|
||||||
|
fStatusCode(0)
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -101,7 +102,10 @@ BUrlResult::operator=(const BUrlResult& other)
|
|||||||
fHeaders = other.fHeaders;
|
fHeaders = other.fHeaders;
|
||||||
|
|
||||||
fRawData.SetSize(other.fRawData.BufferLength());
|
fRawData.SetSize(other.fRawData.BufferLength());
|
||||||
fRawData.WriteAt(0, fRawData.Buffer(), fRawData.BufferLength());
|
fRawData.WriteAt(0, other.fRawData.Buffer(), other.fRawData.BufferLength());
|
||||||
|
// FIXME this makes a copy of the data, it would be better to share it
|
||||||
|
|
||||||
|
fStatusCode = other.fStatusCode;
|
||||||
|
|
||||||
return *this;
|
return *this;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -15,7 +15,7 @@
|
|||||||
|
|
||||||
BUrlSynchronousRequest::BUrlSynchronousRequest(BUrlRequest& request)
|
BUrlSynchronousRequest::BUrlSynchronousRequest(BUrlRequest& request)
|
||||||
:
|
:
|
||||||
BUrlRequest(request.Url(), this, request.Context(), request.Result(),
|
BUrlRequest(request.Url(), NULL, request.Context(),
|
||||||
"BUrlSynchronousRequest", request.Protocol()),
|
"BUrlSynchronousRequest", request.Protocol()),
|
||||||
fRequestComplete(false),
|
fRequestComplete(false),
|
||||||
fWrappedRequest(request)
|
fWrappedRequest(request)
|
||||||
@@ -45,6 +45,8 @@ BUrlSynchronousRequest::WaitUntilCompletion()
|
|||||||
while (!fRequestComplete)
|
while (!fRequestComplete)
|
||||||
snooze(10000);
|
snooze(10000);
|
||||||
|
|
||||||
|
fResult = fWrappedRequest.Result();
|
||||||
|
|
||||||
return B_OK;
|
return B_OK;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user