BewareMyPower commented on code in PR #17452:
URL: https://github.com/apache/pulsar/pull/17452#discussion_r963024003


##########
pulsar-client-cpp/lib/c/c_Consumer.cc:
##########
@@ -60,6 +60,20 @@ pulsar_result 
pulsar_consumer_receive_with_timeout(pulsar_consumer_t *consumer,
     return (pulsar_result)res;
 }
 
+static void handle_receive_callback(pulsar::Result result, pulsar::Message 
message,
+                                    pulsar_receive_callback callback, void 
*ctx) {
+    if (callback) {
+        pulsar_message_t *msg = new pulsar_message_t;
+        msg->message = message;
+        callback((pulsar_result)result, msg, ctx);

Review Comment:
   I think it's better to delete `pulsar_message_t` here and don't leave users 
to call `pulsar_message_free`. We should encourage users to call functions with 
`pulsar_message_` prefix in the callback and pass the results to `ctx`.
   
   For a C user, it's more natural to write code like:
   
   ```c++
   struct receive_ctx {
       pulsar_result result;
       const char *value;
       uint32_t length;
   };
   
   static void receive_callback(pulsar_result result, pulsar_message_t *msg, 
void *ctx) {
       struct receive_ctx *receive_ctx = (struct receive_ctx *)ctx;
       receive_ctx->result = result;
       if (result == pulsar_result_Ok) {
           receive_ctx->value = (const char *)pulsar_message_get_data(msg);
           receive_ctx->length = pulsar_message_get_length(msg);
       } else {
           receive_ctx->value = NULL;
           receive_ctx->length = 0;
       }
   }
   ```
   
   But for the current design, he must save the `msg` in the `ctx` or a global 
variable, then call `pulsar_message_free` after that.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to