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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
