You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by "xuzhenbao (via GitHub)" <gi...@apache.org> on 2026/05/25 13:14:00 UTC

[PR] Introduces binary type value for celix_properties_t. (celix)

xuzhenbao opened a new pull request, #841:
URL: https://github.com/apache/celix/pull/841

   In order to enable event admin to transmit arbitrary messages via the "payload" property entry, this pull request introduces a binary type value for celix_properties_t. And the serialization of binary data is achieved through the base64 encoding.


-- 
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-unsubscribe@celix.apache.org

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


Re: [PR] Introduces binary type value for celix_properties_t. (celix)

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on PR #841:
URL: https://github.com/apache/celix/pull/841#issuecomment-4538647572

   As for binary payload in event admin, users can use whatever library (or their own abstraction over embedTLS/OpenSSL) to do base64 encoding/decoding themselves without incurring the above costs.


-- 
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-unsubscribe@celix.apache.org

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


Re: [PR] Introduces binary type value for celix_properties_t. (celix)

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #841:
URL: https://github.com/apache/celix/pull/841#discussion_r3300507366


##########
conanfile.py:
##########
@@ -369,6 +370,7 @@ def requirements(self):
         if self.options.build_utils:
             self.requires("libzip/[>=1.7.3 <2.0.0]")
             self.requires("libuv/[>=1.49.2 <2.0.0]")
+            self.requires("openssl/[>=3.2.0]")

Review Comment:
   Introducing such a heavy dependency for a simple task like base64 is a no-go. What if downstream users want to use embedTLS to reduce footprint?



-- 
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-unsubscribe@celix.apache.org

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


Re: [PR] Introduces binary type value for celix_properties_t. (celix)

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng closed pull request #841: Introduces binary type value for celix_properties_t.
URL: https://github.com/apache/celix/pull/841


-- 
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-unsubscribe@celix.apache.org

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


Re: [PR] Introduces binary type value for celix_properties_t. (celix)

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #841:
URL: https://github.com/apache/celix/pull/841#discussion_r3300497909


##########
libs/utils/src/properties.c:
##########
@@ -144,6 +144,8 @@ static celix_status_t celix_properties_fillEntry(celix_properties_t* properties,
         entry->value = entry->typed.boolValue ? CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
     } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_ARRAY_LIST) {
         entry->value = celix_utils_arrayListToString(entry->typed.arrayValue);
+    } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_BINARY) {
+        entry->value = celix_utils_binaryToBase64String(entry->typed.binaryValue.data, entry->typed.binaryValue.size);

Review Comment:
   For large binary data, twice the space will be too much a cost.



##########
conanfile.py:
##########
@@ -369,6 +370,7 @@ def requirements(self):
         if self.options.build_utils:
             self.requires("libzip/[>=1.7.3 <2.0.0]")
             self.requires("libuv/[>=1.49.2 <2.0.0]")
+            self.requires("openssl/[>=3.2.0]")

Review Comment:
   Introducing such a heavy dependency for a simple task like base64 is a no-go. What if the downstream user want to use embedTLS to reduce footprint?



##########
libs/utils/include/celix_properties.h:
##########
@@ -85,6 +87,10 @@ typedef struct celix_properties_entry {
         const celix_version_t* versionValue; /**< The Celix version value of the entry. */
         const celix_array_list_t*
             arrayValue; /**< The array list of longs, doubles, bools, strings or versions value of the entry. */
+        struct {
+            const void* data;                /**< The binary data of the entry. */
+            size_t size;                     /**< The size of the binary data. */

Review Comment:
   On 64bit platform, it will cost 8 bytes, and in almost all cases, the upper 4 bytes are all zeros. Too much a cost.



-- 
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-unsubscribe@celix.apache.org

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