You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "empiredan (via GitHub)" <gi...@apache.org> on 2024/08/03 13:54:42 UTC
[PR] fix(test): fix heap-use-after-free error by mutation_batch::add_mutation_if_valid [incubator-pegasus]
empiredan opened a new pull request, #2088:
URL: https://github.com/apache/incubator-pegasus/pull/2088
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
Re: [PR] fix(duplication): fix the dangling pointer after `blob` object is moved [incubator-pegasus]
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on PR #2088:
URL: https://github.com/apache/incubator-pegasus/pull/2088#issuecomment-2284340732
Good work! Could you please rebase the latest master branch to check if there are any clang-tidy warnings or errors.
You don't have to fix all the warnings/errors, some rules can be disabled after discussion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
Re: [PR] fix(duplication): fix the dangling pointer after `blob` object is moved [incubator-pegasus]
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan merged PR #2088:
URL: https://github.com/apache/incubator-pegasus/pull/2088
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
Re: [PR] fix(duplication): fix heap-use-after-free error by mutation_batch::add_mutation_if_valid [incubator-pegasus]
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #2088:
URL: https://github.com/apache/incubator-pegasus/pull/2088#discussion_r1711274616
##########
src/replica/duplication/mutation_batch.cpp:
##########
@@ -196,10 +196,10 @@ void mutation_batch::add_mutation_if_valid(mutation_ptr &mu, decree start_decree
}
blob bb;
- if (update.data.buffer() != nullptr) {
+ if (update.data.buffer()) {
+ // ATTENTION: instead of copy, move could optimize the performance. However, this
+ // would nullify the elements of mu->data.updates.
bb = std::move(update.data);
- } else {
- bb = blob::create_from_bytes(update.data.data(), update.data.length());
}
Review Comment:
While it's possible the `_holder` is nullptr and `_data` has a valid data pointer, so it's needed to process the latter case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
Re: [PR] fix(duplication): fix the dangling pointer after `blob` object is moved [incubator-pegasus]
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #2088:
URL: https://github.com/apache/incubator-pegasus/pull/2088#discussion_r1714062383
##########
src/utils/blob.h:
##########
@@ -119,11 +162,12 @@ class blob
const char *buffer_ptr() const { return _holder.get(); }
- // offset can be negative for buffer dereference
+ // `offset` can be negative for buffer dereference.
blob range(int offset) const
{
- // offset cannot exceed the current length value
- assert(offset <= static_cast<int>(_length));
+ CHECK_LE_MSG(offset,
Review Comment:
These functions maybe on hot paths, do you think it make sense to use DCHECK* macros?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org