Skip to content

A
angle
Commit ce95d05b by Tibor den Ouden Committed by Jamie Madill
Options

Fixed memory leak in egl::Error and gl::Error

Move assignment operator of both classes contained a memory leak. Changed type of mMessage from mutable std::string *to mutable std::unique_ptr<std::string> BUG=angleproject:1264 Change-Id: I7d1419b2e9f385b36afebfd6371983be33ee9e61 Reviewed-on: https://chromium-review.googlesource.com/319520Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Tryjob-Request: Tibor Ouden, den <tibordenouden@gmail.com> Tested-by: 's avatarJamie Madill <jmadill@chromium.org>
parent f786c2b9
Showing with 37 additions and 62 deletions
src/libANGLE/Error.cpp
...@@ -16,8 +16,7 @@ ...@@ -16,8 +16,7 @@
namespace gl namespace gl
{ {
Error::Error(GLenum errorCode, const char *msg, ...) Error::Error(GLenum errorCode, const char *msg, ...) : mCode(errorCode), mID(errorCode)
: mCode(errorCode), mID(errorCode), mMessage(nullptr)
{ {
va_list vararg; va_list vararg;
va_start(vararg, msg); va_start(vararg, msg);
...@@ -26,8 +25,7 @@ Error::Error(GLenum errorCode, const char *msg, ...) ...@@ -26,8 +25,7 @@ Error::Error(GLenum errorCode, const char *msg, ...)
va_end(vararg); va_end(vararg);
} }
Error::Error(GLenum errorCode, GLuint id, const char *msg, ...) Error::Error(GLenum errorCode, GLuint id, const char *msg, ...) : mCode(errorCode), mID(id)
: mCode(errorCode), mID(id), mMessage(nullptr)
{ {
va_list vararg; va_list vararg;
va_start(vararg, msg); va_start(vararg, msg);
...@@ -38,9 +36,9 @@ Error::Error(GLenum errorCode, GLuint id, const char *msg, ...) ...@@ -38,9 +36,9 @@ Error::Error(GLenum errorCode, GLuint id, const char *msg, ...)
void Error::createMessageString() const void Error::createMessageString() const
{ {
if (mMessage == nullptr) if (!mMessage)
{ {
mMessage = new std::string(); mMessage.reset(new std::string);
} }
} }
...@@ -56,8 +54,7 @@ bool Error::operator==(const Error &other) const ...@@ -56,8 +54,7 @@ bool Error::operator==(const Error &other) const
return false; return false;
// TODO(jmadill): Compare extended error codes instead of strings. // TODO(jmadill): Compare extended error codes instead of strings.
if ((mMessage == nullptr || other.mMessage == nullptr) && if ((!mMessage || !other.mMessage) && (!mMessage != !other.mMessage))
((mMessage == nullptr) != (other.mMessage == nullptr)))
return false; return false;
return (*mMessage == *other.mMessage); return (*mMessage == *other.mMessage);
...@@ -72,10 +69,7 @@ bool Error::operator!=(const Error &other) const ...@@ -72,10 +69,7 @@ bool Error::operator!=(const Error &other) const
namespace egl namespace egl
{ {
Error::Error(EGLint errorCode, const char *msg, ...) Error::Error(EGLint errorCode, const char *msg, ...) : mCode(errorCode), mID(0)
: mCode(errorCode),
mID(0),
mMessage(nullptr)
{ {
va_list vararg; va_list vararg;
va_start(vararg, msg); va_start(vararg, msg);
...@@ -84,10 +78,7 @@ Error::Error(EGLint errorCode, const char *msg, ...) ...@@ -84,10 +78,7 @@ Error::Error(EGLint errorCode, const char *msg, ...)
va_end(vararg); va_end(vararg);
} }
Error::Error(EGLint errorCode, EGLint id, const char *msg, ...) Error::Error(EGLint errorCode, EGLint id, const char *msg, ...) : mCode(errorCode), mID(id)
: mCode(errorCode),
mID(id),
mMessage(nullptr)
{ {
va_list vararg; va_list vararg;
va_start(vararg, msg); va_start(vararg, msg);
...@@ -95,11 +86,12 @@ Error::Error(EGLint errorCode, EGLint id, const char *msg, ...) ...@@ -95,11 +86,12 @@ Error::Error(EGLint errorCode, EGLint id, const char *msg, ...)
*mMessage = FormatString(msg, vararg); *mMessage = FormatString(msg, vararg);
va_end(vararg); va_end(vararg);
} }
void Error::createMessageString() const void Error::createMessageString() const
{ {
if (mMessage == nullptr) if (!mMessage)
{ {
mMessage = new std::string(); mMessage.reset(new std::string);
} }
} }
......
src/libANGLE/Error.h
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include <EGL/egl.h> #include <EGL/egl.h>
#include <string> #include <string>
#include <memory>
namespace gl namespace gl
{ {
...@@ -26,8 +27,6 @@ class Error final ...@@ -26,8 +27,6 @@ class Error final
inline Error(const Error &other); inline Error(const Error &other);
inline Error(Error &&other); inline Error(Error &&other);
inline ~Error();
inline Error &operator=(const Error &other); inline Error &operator=(const Error &other);
inline Error &operator=(Error &&other); inline Error &operator=(Error &&other);
...@@ -46,7 +45,7 @@ class Error final ...@@ -46,7 +45,7 @@ class Error final
GLenum mCode; GLenum mCode;
GLuint mID; GLuint mID;
mutable std::string *mMessage; mutable std::unique_ptr<std::string> mMessage;
}; };
} }
...@@ -63,8 +62,6 @@ class Error final ...@@ -63,8 +62,6 @@ class Error final
inline Error(const Error &other); inline Error(const Error &other);
inline Error(Error &&other); inline Error(Error &&other);
inline ~Error();
inline Error &operator=(const Error &other); inline Error &operator=(const Error &other);
inline Error &operator=(Error &&other); inline Error &operator=(Error &&other);
...@@ -79,7 +76,7 @@ class Error final ...@@ -79,7 +76,7 @@ class Error final
EGLint mCode; EGLint mCode;
EGLint mID; EGLint mID;
mutable std::string *mMessage; mutable std::unique_ptr<std::string> mMessage;
}; };
} }
......
src/libANGLE/Error.inl
...@@ -15,17 +15,15 @@ namespace gl ...@@ -15,17 +15,15 @@ namespace gl
Error::Error(GLenum errorCode) Error::Error(GLenum errorCode)
: mCode(errorCode), : mCode(errorCode),
mID(errorCode), mID(errorCode)
mMessage(nullptr)
{ {
} }
Error::Error(const Error &other) Error::Error(const Error &other)
: mCode(other.mCode), : mCode(other.mCode),
mID(other.mID), mID(other.mID)
mMessage(nullptr)
{ {
if (other.mMessage != nullptr) if (other.mMessage)
{ {
createMessageString(); createMessageString();
*mMessage = *(other.mMessage); *mMessage = *(other.mMessage);
...@@ -35,14 +33,8 @@ Error::Error(const Error &other) ...@@ -35,14 +33,8 @@ Error::Error(const Error &other)
Error::Error(Error &&other) Error::Error(Error &&other)
: mCode(other.mCode), : mCode(other.mCode),
mID(other.mID), mID(other.mID),
mMessage(other.mMessage) mMessage(std::move(other.mMessage))
{ {
other.mMessage = nullptr;
}
Error::~Error()
{
SafeDelete(mMessage);
} }
Error &Error::operator=(const Error &other) Error &Error::operator=(const Error &other)
...@@ -50,14 +42,14 @@ Error &Error::operator=(const Error &other) ...@@ -50,14 +42,14 @@ Error &Error::operator=(const Error &other)
mCode = other.mCode; mCode = other.mCode;
mID = other.mID; mID = other.mID;
if (other.mMessage != nullptr) if (other.mMessage)
{ {
createMessageString(); createMessageString();
*mMessage = *(other.mMessage); *mMessage = *(other.mMessage);
} }
else else
{ {
SafeDelete(mMessage); mMessage.release();
} }
return *this; return *this;
...@@ -65,11 +57,12 @@ Error &Error::operator=(const Error &other) ...@@ -65,11 +57,12 @@ Error &Error::operator=(const Error &other)
Error &Error::operator=(Error &&other) Error &Error::operator=(Error &&other)
{ {
mCode = other.mCode; if (this != &other)
mID = other.mID; {
mMessage = other.mMessage; mCode = other.mCode;
mID = other.mID;
other.mMessage = nullptr; mMessage = std::move(other.mMessage);
}
return *this; return *this;
} }
...@@ -96,17 +89,15 @@ namespace egl ...@@ -96,17 +89,15 @@ namespace egl
Error::Error(EGLint errorCode) Error::Error(EGLint errorCode)
: mCode(errorCode), : mCode(errorCode),
mID(0), mID(0)
mMessage(nullptr)
{ {
} }
Error::Error(const Error &other) Error::Error(const Error &other)
: mCode(other.mCode), : mCode(other.mCode),
mID(other.mID), mID(other.mID)
mMessage(nullptr)
{ {
if (other.mMessage != nullptr) if (other.mMessage)
{ {
createMessageString(); createMessageString();
*mMessage = *(other.mMessage); *mMessage = *(other.mMessage);
...@@ -116,14 +107,8 @@ Error::Error(const Error &other) ...@@ -116,14 +107,8 @@ Error::Error(const Error &other)
Error::Error(Error &&other) Error::Error(Error &&other)
: mCode(other.mCode), : mCode(other.mCode),
mID(other.mID), mID(other.mID),
mMessage(other.mMessage) mMessage(std::move(other.mMessage))
{ {
other.mMessage = nullptr;
}
Error::~Error()
{
SafeDelete(mMessage);
} }
Error &Error::operator=(const Error &other) Error &Error::operator=(const Error &other)
...@@ -131,14 +116,14 @@ Error &Error::operator=(const Error &other) ...@@ -131,14 +116,14 @@ Error &Error::operator=(const Error &other)
mCode = other.mCode; mCode = other.mCode;
mID = other.mID; mID = other.mID;
if (other.mMessage != nullptr) if (other.mMessage)
{ {
createMessageString(); createMessageString();
*mMessage = *(other.mMessage); *mMessage = *(other.mMessage);
} }
else else
{ {
SafeDelete(mMessage); mMessage.release();
} }
return *this; return *this;
...@@ -146,11 +131,12 @@ Error &Error::operator=(const Error &other) ...@@ -146,11 +131,12 @@ Error &Error::operator=(const Error &other)
Error &Error::operator=(Error &&other) Error &Error::operator=(Error &&other)
{ {
mCode = other.mCode; if (this != &other)
mID = other.mID; {
mMessage = other.mMessage; mCode = other.mCode;
mID = other.mID;
other.mMessage = nullptr; mMessage = std::move(other.mMessage);
}
return *this; return *this;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment