celix-dev mailing list archives

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

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



##########
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:
       Why is this one added?

##########
File path: bundles/pubsub/pubsub_admin_zmq/src/pubsub_zmq_topic_receiver.c
##########
@@ -650,6 +650,8 @@ static void* psa_zmq_recvThread(void * data) {
                 } else {
                     message.metadata.metadata = NULL;
                 }
+                zframe_t *footer = zmsg_pop(zmsg); // footer

Review comment:
       move this one up to the other frames?

##########
File path: bundles/pubsub/pubsub_admin_zmq/src/pubsub_zmq_topic_receiver.c
##########
@@ -650,6 +650,8 @@ static void* psa_zmq_recvThread(void * data) {
                 } else {
                     message.metadata.metadata = NULL;
                 }
+                zframe_t *footer = zmsg_pop(zmsg); // footer
+                receiver->protocol->decodeFooter(receiver->protocol->handle,
zframe_data(footer), zframe_size(footer), &message);

Review comment:
       The footer is not used here? Also, currently the payload and metadatasize is part of
the header. Since the footer only has a sync byte, the size is not needed?

##########
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:
       Why is freeMsgEntry removed here?

##########
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:
       Wrong comment




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