celix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [celix] rbulter commented on a change in pull request #253: Feature/proposal protocol footer
Date Thu, 11 Jun 2020 18:25:27 GMT

rbulter commented on a change in pull request #253:
URL: https://github.com/apache/celix/pull/253#discussion_r438984082



##########
File path: bundles/pubsub/pubsub_admin_zmq/src/pubsub_zmq_topic_sender.c
##########
@@ -635,13 +631,24 @@ static int psa_zmq_topicPublicationSend(void* handle, unsigned int msgTypeId,
co
 
                     //send MetaData
                     if (rc > 0 && metadataLength > 0) {
-                        zmq_msg_init_data(&msg3, metadataData, metadataLength, psa_zmq_freeMsg,
freeMsgEntry);
-                        rc = zmq_msg_send(&msg3, socket, 0);
+                        zmq_msg_init_data(&msg3, metadataData, metadataLength, psa_zmq_freeMsg,
NULL);

Review comment:
       Because this is not correct.
   According zmq_msg_init_data, the zmq_free_fn *ffn is called when the data is not needed
anymore. The freeMsgEntry is hint to remove serialized output of the payload.
   When the metadata is not needed anymore the metadata should be removed, not the serialized
data.

##########
File path: bundles/pubsub/pubsub_protocol_wire_v1/src/pubsub_wire_protocol_impl.c
##########
@@ -175,6 +176,27 @@ celix_status_t pubsubProtocol_encodeMetadata(void *handle, pubsub_protocol_messa
     return status;
 }
 
+celix_status_t pubsubProtocol_encodeFooter(void *handle, pubsub_protocol_message_t *message,
void **outBuffer, size_t *outLength) {
+    celix_status_t status = CELIX_SUCCESS;
+    // Get HeaderSize

Review comment:
       I will fix this

##########
File path: libs/framework/src/celix_log.c
##########
@@ -26,6 +26,7 @@
 #include "celix_log.h"
 #include "celix_threads.h"
 #include "celix_array_list.h"
+#include "memstream/open_memstream.h"

Review comment:
       When compiling under OSX i had issue that mem_stream in celix_log cannot be found resulting
into build error using Clion




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message