PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1377595646
##########
bundles/remote_services/discovery_zeroconf/src/discovery_zeroconf_announcer.c:
##########
@@ -342,11 +342,9 @@ static void
discoveryZeroconfAnnouncer_revokeEndpoints(discovery_zeroconf_announ
static bool
discoveryZeroconfAnnouncer_copyPropertiesToTxtRecord(discovery_zeroconf_announcer_t
*announcer, celix_properties_iterator_t *propIter, TXTRecordRef *txtRecord,
uint16_t maxTxtLen, bool splitTxtRecord) {
const char *key;
const char *val;
- celix_properties_t *props;
- while (celix_propertiesIterator_hasNext(propIter)) {
- key = celix_propertiesIterator_nextKey(propIter);
- props = celix_propertiesIterator_properties(propIter);
- val = celix_properties_get(props, key, "");
+ while (!celix_propertiesIterator_isEnd(propIter)) {
+ key = propIter->key;
+ val = propIter->entry.value;
Review Comment:
Shall we check whether `val == NULL`?
##########
bundles/remote_services/discovery_common/src/endpoint_descriptor_writer.c:
##########
@@ -139,20 +139,15 @@ static celix_status_t
endpointDescriptorWriter_writeEndpoint(endpoint_descriptor
} else {
xmlTextWriterStartElement(writer->writer, ENDPOINT_DESCRIPTION);
- hash_map_iterator_pt iter =
hashMapIterator_create(endpoint->properties);
- while (hashMapIterator_hasNext(iter)) {
- hash_map_entry_pt entry = hashMapIterator_nextEntry(iter);
-
- void* propertyName = hashMapEntry_getKey(entry);
- const xmlChar* propertyValue = (const xmlChar*)
hashMapEntry_getValue(entry);
-
+ CELIX_PROPERTIES_ITERATE(endpoint->properties, iter) {
+ const xmlChar* propertyValue = (const xmlChar*)
celix_properties_get(endpoint->properties, iter.key, "");
Review Comment:
Why not `iter.entry.value`? Maybe the value could be `NULL`?
If value could be `NULL`, we need to check all previous usages of
`iter.entry.value`.
##########
bundles/pubsub/pubsub_protocol/pubsub_protocol_lib/src/pubsub_wire_protocol_common.c:
##########
@@ -101,19 +101,17 @@ celix_status_t
pubsubProtocol_encodeMetadata(pubsub_protocol_message_t* message,
*bufferInOut = newBuffer;
*bufferLengthInOut = newLength;
}
- const char* key;
if (metadataSize == 0) {
encoded = true;
continue;
}
celix_status_t status = CELIX_SUCCESS;
- CELIX_PROPERTIES_FOR_EACH(message->metadata.metadata, key) {
- const char *val = celix_properties_get(message->metadata.metadata,
key, "");
+ CELIX_PROPERTIES_ITERATE(message->metadata.metadata, iter) {
if (status == CELIX_SUCCESS) {
- status =
pubsubProtocol_addNetstringEntryToBuffer(*bufferInOut, *bufferLengthInOut,
&offset, key);
+ status =
pubsubProtocol_addNetstringEntryToBuffer(*bufferInOut, *bufferLengthInOut,
&offset, iter.key);
Review Comment:
Previously, it was guaranteed that `val != NULL`. Now does this invariant
still hold?
##########
bundles/remote_services/discovery_zeroconf/src/discovery_zeroconf_announcer.c:
##########
@@ -425,6 +424,7 @@ static void
discoveryZeroconfAnnouncer_announceEndpoints(discovery_zeroconf_anno
break;
}
TXTRecordDeallocate(&txtRecord);
+ celix_propertiesIterator_next(&propIter);
Review Comment:
`discoveryZeroconfAnnouncer_copyPropertiesToTxtRecord` already calls
`celix_propertiesIterator_next` once. @xuzhenbao please have a look at RSA.
--
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: dev-unsubscr...@celix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
