You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2019/03/01 16:28:36 UTC

[GitHub] nkurihar opened a new pull request #3729: fix pulsar_message_set_replication_clusters

nkurihar opened a new pull request #3729: fix pulsar_message_set_replication_clusters
URL: https://github.com/apache/pulsar/pull/3729
 
 
   ### Motivation
   `pulsar_message_set_replication_clusters()` in `c_Message.cc` doesn't work.
   
   To catch what happens, I executed [the small test code](https://gist.github.com/nkurihar/474ddba7bf2598730bb94ebbb8442117) and saw the infinite loop and finally segmentation fault:
   ```
   c1
   1
   
   c2
   2
   
   c3
   3
   
   ?
   ?
   ;?
   ;?
   ?
   
   <
   
   x???8
   ???8
   ??8
   ?8
   8
   
   .
   .
   .
   
   Segmentation fault
   ```
   
   ### Modifications
   - Fix the type of the variable `c`: it needs to be not `const char*` but `const char**`
   - Add the third argument `size` to judge when to stop the loop
   
   ### Verifying this change
   - [ ] Make sure that the change passes the CI checks.
   
   ### Does this pull request potentially affect one of the following parts:
     - The public API: yes
   
   This PR changes the interface of `pulsar_message_set_replication_clusters()`.
   If we want to keep the back compatibility, another option is to add `NULL` to `clusters` as the last element before passing to `pulsar_message_set_replication_clusters()`, and judging when to stop the loop by null check (like `while (*c) { ... }`) in the function.
   However, I think to force users to add `NULL` as the last element is error-prone, instead adding the size argument is natural.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services