pulsar-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] nkurihar opened a new pull request #3729: fix pulsar_message_set_replication_clusters
Date Fri, 01 Mar 2019 16:28:36 GMT
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:
   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:

With regards,
Apache Git Services

View raw message