You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "Sunflower876 (via GitHub)" <gi...@apache.org> on 2024/03/08 07:57:17 UTC

[PR] fix(message_ex): In some case, the message_ex::copy function will trigger an assertion. Fix it. [incubator-pegasus]

Sunflower876 opened a new pull request, #1938:
URL: https://github.com/apache/incubator-pegasus/pull/1938

   ### What problem does this PR solve? <!--add issue link with summary if exists-->
   #1937
   
   ##### Tests <!-- At least one of them must be included. -->
   
   - Unit test
   - Manual test (add detailed scripts or steps below)
   
   


-- 
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(message_ex): In some case, the message_ex::copy function will trigger an assertion. Fix it. [incubator-pegasus]

Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1938:
URL: https://github.com/apache/incubator-pegasus/pull/1938#discussion_r1555687643


##########
src/runtime/rpc/rpc_message.cpp:
##########
@@ -252,6 +252,7 @@ message_ex *message_ex::copy(bool clone_content, bool copy_for_receive)
 
         if ((const char *)header != buffers[0].data()) {
             memcpy(ptr, (const void *)header, sizeof(message_header));
+            i += sizeof(message_header);

Review Comment:
   Thanks for the contribution!



-- 
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(message_ex): In some case, the message_ex::copy function will trigger an assertion. Fix it. [incubator-pegasus]

Posted by "Sunflower876 (via GitHub)" <gi...@apache.org>.
Sunflower876 commented on code in PR #1938:
URL: https://github.com/apache/incubator-pegasus/pull/1938#discussion_r1528472798


##########
src/runtime/rpc/rpc_message.cpp:
##########
@@ -252,6 +252,7 @@ message_ex *message_ex::copy(bool clone_content, bool copy_for_receive)
 
         if ((const char *)header != buffers[0].data()) {
             memcpy(ptr, (const void *)header, sizeof(message_header));
+            i += sizeof(message_header);

Review Comment:
   I have made the changes, could you please help me take a look again.



-- 
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(message_ex): In some case, the message_ex::copy function will trigger an assertion. Fix it. [incubator-pegasus]

Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1938:
URL: https://github.com/apache/incubator-pegasus/pull/1938#discussion_r1519282465


##########
src/runtime/rpc/rpc_message.cpp:
##########
@@ -252,6 +252,7 @@ message_ex *message_ex::copy(bool clone_content, bool copy_for_receive)
 
         if ((const char *)header != buffers[0].data()) {
             memcpy(ptr, (const void *)header, sizeof(message_header));
+            i += sizeof(message_header);

Review Comment:
   How about removing the `i` variable totally, you can compare the offset of `ptr` and `recv_buffer` instead in line 264.



-- 
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(message_ex): Fix the crash caused by message_ex::copy() in server internally [incubator-pegasus]

Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan merged PR #1938:
URL: https://github.com/apache/incubator-pegasus/pull/1938


-- 
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