Introduces binary type value for celix_properties_t.#841
Conversation
PengZheng
left a comment
PengZheng
left a comment
There was a problem hiding this comment.
The current implementation adds too much cost, as the following describes:
| 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. */ |
There was a problem hiding this 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.
| } 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); |
There was a problem hiding this comment.
For large binary data, twice the space will be too much a cost.
| 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]") |
There was a problem hiding this 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?
|
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. Or event admin can extend |
2 participants
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.