-
Notifications
You must be signed in to change notification settings - Fork 581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix assignment operator issues in ByteArray #51
base: main
Are you sure you want to change the base?
Conversation
noSTALKER
commented
May 24, 2019
- Fix assignment operator issues in ByteArray
- Add noexcept qualifier to functions
+ Fix assignment operator issues in ByteArray + Add noexcept qualifier to functions
m_length(0) | ||
ByteArray::ByteArray() noexcept | ||
: m_data(nullptr) | ||
, m_length(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please keep original formatting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have reverted the changes done in member initializer lists
m_data = p_right.m_data; | ||
m_length = p_right.m_length; | ||
m_dataHolder = p_right.m_dataHolder; | ||
if (this != std::addressof(p_right)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think shared_ptr has already handled such case? Not sure if this is necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't checked yet but it is just a recommended standard check in both copy constructor and move constructors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take some investigation and think this check is not necessary for std::shared_ptr and most other stl containers (it seems a popular implementation for shared_ptr assign function is to create a temp var and then swap it with this
). So please remove this check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have reverted back the checks
The base branch was changed.
ByteArray& | ||
ByteArray::operator= (ByteArray&& p_right) | ||
ByteArray::operator=(ByteArray&& p_right) noexcept | ||
{ | ||
m_data = p_right.m_data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this move too?