Re: [PR] type support for properties (celix)
pnoltes merged PR #470: URL: https://github.com/apache/celix/pull/470 -- 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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1399944798
##
libs/utils/src/properties.c:
##
@@ -191,324 +476,423 @@ static void parseLine(const char* line,
celix_properties_t *props) {
}
if (!isComment) {
-//printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
+// printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
celix_properties_set(props, celix_utils_trimInPlace(key),
celix_utils_trimInPlace(value));
}
-if(key) {
+if (key) {
free(key);
-}
-if(value) {
free(value);
}
-
}
+celix_properties_t* celix_properties_loadWithStream(FILE* file) {
+if (file == NULL) {
+return NULL;
+}
-/**
-
**
- * Updated API
-
**
-
**/
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+if (!props) {
+celix_err_push("Failed to create properties");
+return NULL;
+}
+int rc = fseek(file, 0, SEEK_END);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to end of file. Got error %i", errno);
+return NULL;
+}
+size_t fileSize = ftell(file);
+rc = fseek(file, 0, SEEK_SET);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to start of file. Got error %i", errno);
+return NULL;
+}
Review Comment:
> Also if we add a max properties file size, I think this should be
configurable with a conan/cmake build option.
I think we can deal with it later when necessary.
##
libs/utils/src/properties.c:
##
@@ -191,324 +476,423 @@ static void parseLine(const char* line,
celix_properties_t *props) {
}
if (!isComment) {
-//printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
+// printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
celix_properties_set(props, celix_utils_trimInPlace(key),
celix_utils_trimInPlace(value));
}
-if(key) {
+if (key) {
free(key);
-}
-if(value) {
free(value);
}
-
}
+celix_properties_t* celix_properties_loadWithStream(FILE* file) {
+if (file == NULL) {
+return NULL;
+}
-/**
-
**
- * Updated API
-
**
-
**/
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+if (!props) {
+celix_err_push("Failed to create properties");
+return NULL;
+}
+int rc = fseek(file, 0, SEEK_END);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to end of file. Got error %i", errno);
+return NULL;
+}
+size_t fileSize = ftell(file);
+rc = fseek(file, 0, SEEK_SET);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to start of file. Got error %i", errno);
+return NULL;
+}
Review Comment:
> Also if we add a max properties file size, I think this should be
configurable with a conan/cmake build option.
I think we can deal with it later when necessary.
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1399584425
##
libs/utils/src/properties.c:
##
@@ -191,324 +476,423 @@ static void parseLine(const char* line,
celix_properties_t *props) {
}
if (!isComment) {
-//printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
+// printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
celix_properties_set(props, celix_utils_trimInPlace(key),
celix_utils_trimInPlace(value));
}
-if(key) {
+if (key) {
free(key);
-}
-if(value) {
free(value);
}
-
}
+celix_properties_t* celix_properties_loadWithStream(FILE* file) {
+if (file == NULL) {
+return NULL;
+}
-/**
-
**
- * Updated API
-
**
-
**/
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+if (!props) {
+celix_err_push("Failed to create properties");
+return NULL;
+}
+int rc = fseek(file, 0, SEEK_END);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to end of file. Got error %i", errno);
+return NULL;
+}
+size_t fileSize = ftell(file);
+rc = fseek(file, 0, SEEK_SET);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to start of file. Got error %i", errno);
+return NULL;
+}
Review Comment:
I think this is wise, but what is a good max properties file size?
Also if we add a max properties file size, I think this should be
configurable with a conan/cmake build option.
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1399582716
##
libs/utils/src/properties.c:
##
@@ -191,324 +476,423 @@ static void parseLine(const char* line,
celix_properties_t *props) {
}
if (!isComment) {
-//printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
+// printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
celix_properties_set(props, celix_utils_trimInPlace(key),
celix_utils_trimInPlace(value));
}
-if(key) {
+if (key) {
free(key);
-}
-if(value) {
free(value);
}
-
}
+celix_properties_t* celix_properties_loadWithStream(FILE* file) {
+if (file == NULL) {
+return NULL;
+}
-/**
-
**
- * Updated API
-
**
-
**/
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+if (!props) {
+celix_err_push("Failed to create properties");
+return NULL;
+}
+int rc = fseek(file, 0, SEEK_END);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to end of file. Got error %i", errno);
+return NULL;
+}
+size_t fileSize = ftell(file);
Review Comment:
added ftell return check and error injection test
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on PR #470: URL: https://github.com/apache/celix/pull/470#issuecomment-1819541539 > > Therefore, it is also reasonable to remove `index` from the interface and implementation. WDYT? I agree. The index field is too confusion and it is trivial to add a counter/index on your own, so I removed the index field. -- 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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398332841
##
libs/utils/src/properties.c:
##
@@ -191,324 +476,423 @@ static void parseLine(const char* line,
celix_properties_t *props) {
}
if (!isComment) {
-//printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
+// printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
celix_properties_set(props, celix_utils_trimInPlace(key),
celix_utils_trimInPlace(value));
}
-if(key) {
+if (key) {
free(key);
-}
-if(value) {
free(value);
}
-
}
+celix_properties_t* celix_properties_loadWithStream(FILE* file) {
+if (file == NULL) {
+return NULL;
+}
-/**
-
**
- * Updated API
-
**
-
**/
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+if (!props) {
+celix_err_push("Failed to create properties");
+return NULL;
+}
+int rc = fseek(file, 0, SEEK_END);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to end of file. Got error %i", errno);
+return NULL;
+}
+size_t fileSize = ftell(file);
+rc = fseek(file, 0, SEEK_SET);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to start of file. Got error %i", errno);
+return NULL;
+}
+char* fileBuffer = malloc(fileSize + 1);
+if (fileBuffer == NULL) {
+celix_err_pushf("Cannot allocate memory for file buffer. Got error
%i", errno);
+return NULL;
+}
-celix_properties_t* celix_properties_create(void) {
-return hashMap_create(utils_stringHash, utils_stringHash,
utils_stringEquals, utils_stringEquals);
-}
+size_t rs = fread(fileBuffer, sizeof(char), fileSize, file);
+if (rs < fileSize) {
+fprintf(stderr, "fread read only %zu bytes out of %zu\n", rs,
fileSize);
+}
+fileBuffer[fileSize] = '\0'; // ensure a '\0' at the end of the fileBuffer
-void celix_properties_destroy(celix_properties_t *properties) {
-if (properties != NULL) {
-hash_map_iterator_pt iter = hashMapIterator_create(properties);
-while (hashMapIterator_hasNext(iter)) {
-hash_map_entry_pt entry = hashMapIterator_nextEntry(iter);
-hashMapEntry_clear(entry, true, true);
-}
-hashMapIterator_destroy(iter);
-hashMap_destroy(properties, false, false);
+char* savePtr = NULL;
+char* line = strtok_r(fileBuffer, "\n", &savePtr);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &savePtr);
}
+free(fileBuffer);
+
+return celix_steal_ptr(props);
}
-celix_properties_t* celix_properties_load(const char *filename) {
-FILE *file = fopen(filename, "r");
-if (file == NULL) {
+celix_properties_t* celix_properties_loadFromString(const char* input) {
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+celix_autofree char* in = celix_utils_strdup(input);
+if (!props || !in) {
+celix_err_push("Failed to create properties or duplicate input
string");
return NULL;
}
-celix_properties_t *props = celix_properties_loadWithStream(file);
-fclose(file);
-return props;
-}
-
-celix_properties_t* celix_properties_loadWithStream(FILE *file) {
-celix_properties_t *props = NULL;
-if (file != NULL ) {
-char *saveptr;
-char *filebuffer = NULL;
-char *line = NULL;
-ssize_t file_size = 0;
-
-props = celix_properties_create();
-fseek(file, 0, SEEK_END);
-file_size = ftell(file);
-fseek(file, 0, SEEK_SET);
+char* line = NULL;
+char* saveLinePointer = NULL;
+line = strtok_r(in, "\n", &saveLinePointer);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &saveLinePointer);
+}
+return celix_steal_ptr(props);
+}
-if (file_size > 0) {
-filebuffer = calloc(file_size + 1, sizeof(char));
-if (filebuffer) {
-size_t rs = fread(filebuffer, sizeof(char), file_size, file);
-if (rs != file_size) {
-fprintf(stderr,"fread read only %lu bytes out of %lu\n",
(long unsigned int) rs, (long unsigned int) file_size);
-}
-filebuffer[file_size]='\0'
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398331655
##
libs/utils/src/properties.c:
##
@@ -117,69 +409,62 @@ static void parseLine(const char* line,
celix_properties_t *props) {
output = NULL;
outputPos = 0;
-//Ignore empty lines
+// Ignore empty lines
if (line[0] == '\n' && line[1] == '\0') {
return;
}
-char *key = calloc(1, key_len);
-char *value = calloc(1, value_len);
+char* key = calloc(1, key_len);
+char* value = calloc(1, value_len);
key[0] = '\0';
value[0] = '\0';
while (line[linePos] != '\0') {
if (line[linePos] == ' ' || line[linePos] == '\t') {
if (output == NULL) {
-//ignore
+// ignore
linePos += 1;
continue;
}
-}
-else {
+} else {
if (output == NULL) {
output = key;
}
}
if (line[linePos] == '=' || line[linePos] == ':' || line[linePos] ==
'#' || line[linePos] == '!') {
if (precedingCharIsBackslash) {
-//escaped special character
+// escaped special character
output[outputPos++] = line[linePos];
updateBuffers(&key, &value, &output, outputPos, &key_len,
&value_len);
Review Comment:
It can be delayed until we solve #685.
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on PR #470:
URL: https://github.com/apache/celix/pull/470#issuecomment-1817754005
> Ah I see, no this was not intentionally and I think it is better to keep
the max iterator index on map size. Refactor the index increment for this.
From the following documentation, I get the original intention of `index`:
```C
typedef struct celix_string_hash_map_iterator {
size_t index;/**< The index of the iterator. Starts at 0, ends at
map size (so beyond the last element). When an
entry is removed, the index value is not changed. */
```
And yes, `celix_stringHashMapIterator_next allows iter->index >
map->genericMap.size` is intentionally allowed.
I noticed the newly added test `TEST_F(HashMapTestSuite,
IterateWithRemoveAtIndex4Test)` seems weird:
```C
celix_autoptr(celix_string_hash_map_t) sMap = createStringHashMap(6);
auto iter1 = celix_stringHashMap_begin(sMap);
while (!celix_stringHashMapIterator_isEnd(&iter1)) {
if (iter1.index == 4) {
// note only removing entries where the iter key is even
celix_stringHashMapIterator_remove(&iter1);
} else {
celix_stringHashMapIterator_next(&iter1);
}
}
EXPECT_EQ(4, celix_stringHashMap_size(sMap));
```
4 rather than 5 entries remain.
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on PR #470: URL: https://github.com/apache/celix/pull/470#issuecomment-1817543557 @PengZheng I think I reworked all remarks. Could you re-review? -- 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]
Re: [PR] type support for properties (celix)
pnoltes commented on PR #470: URL: https://github.com/apache/celix/pull/470#issuecomment-1817518370 > I agree that "the index of the end iterator is the same as the size of the map". The current implementation of celix_stringHashMapIterator_next allows iter->index > map->genericMap.size. Is it intended? Ah i see, no this was not intentionally and I think it is better to keep the max iterator index on map size. Refactor the index increment for this. -- 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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398214396
##
libs/utils/src/properties.c:
##
@@ -191,324 +481,449 @@ static void parseLine(const char* line,
celix_properties_t *props) {
}
if (!isComment) {
-//printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
+// printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
celix_properties_set(props, celix_utils_trimInPlace(key),
celix_utils_trimInPlace(value));
}
-if(key) {
+if (key) {
free(key);
-}
-if(value) {
free(value);
}
-
}
+celix_properties_t* celix_properties_loadWithStream(FILE* file) {
+if (file == NULL) {
+return NULL;
+}
-/**
-
**
- * Updated API
-
**
-
**/
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+if (!props) {
+celix_err_push("Failed to create properties");
+return NULL;
+}
+int rc = fseek(file, 0, SEEK_END);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to end of file. Got error %i", errno);
+return NULL;
+}
+size_t fileSize = ftell(file);
+rc = fseek(file, 0, SEEK_SET);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to start of file. Got error %i", errno);
+return NULL;
+}
+char* fileBuffer = malloc(fileSize + 1);
+if (fileBuffer == NULL) {
+celix_err_pushf("Cannot allocate memory for file buffer. Got error
%i", errno);
+return NULL;
+}
-celix_properties_t* celix_properties_create(void) {
-return hashMap_create(utils_stringHash, utils_stringHash,
utils_stringEquals, utils_stringEquals);
-}
+size_t rs = fread(fileBuffer, sizeof(char), fileSize, file);
+if (rs < fileSize) {
+fprintf(stderr, "fread read only %zu bytes out of %zu\n", rs,
fileSize);
+}
+fileBuffer[fileSize] = '\0'; // ensure a '\0' at the end of the fileBuffer
-void celix_properties_destroy(celix_properties_t *properties) {
-if (properties != NULL) {
-hash_map_iterator_pt iter = hashMapIterator_create(properties);
-while (hashMapIterator_hasNext(iter)) {
-hash_map_entry_pt entry = hashMapIterator_nextEntry(iter);
-hashMapEntry_clear(entry, true, true);
-}
-hashMapIterator_destroy(iter);
-hashMap_destroy(properties, false, false);
+char* savePtr = NULL;
+char* line = strtok_r(fileBuffer, "\n", &savePtr);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &savePtr);
}
+free(fileBuffer);
+
+return celix_steal_ptr(props);
}
-celix_properties_t* celix_properties_load(const char *filename) {
-FILE *file = fopen(filename, "r");
-if (file == NULL) {
+celix_properties_t* celix_properties_loadFromString(const char* input) {
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+celix_autofree char* in = celix_utils_strdup(input);
+if (!props || !in) {
+celix_err_push("Failed to create properties or duplicate input
string");
return NULL;
}
-celix_properties_t *props = celix_properties_loadWithStream(file);
-fclose(file);
-return props;
-}
-celix_properties_t* celix_properties_loadWithStream(FILE *file) {
-celix_properties_t *props = NULL;
-
-if (file != NULL ) {
-char *saveptr;
-char *filebuffer = NULL;
-char *line = NULL;
-ssize_t file_size = 0;
-
-props = celix_properties_create();
-fseek(file, 0, SEEK_END);
-file_size = ftell(file);
-fseek(file, 0, SEEK_SET);
+char* line = NULL;
+char* saveLinePointer = NULL;
+line = strtok_r(in, "\n", &saveLinePointer);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &saveLinePointer);
+}
+return celix_steal_ptr(props);
+}
-if (file_size > 0) {
-filebuffer = calloc(file_size + 1, sizeof(char));
-if (filebuffer) {
-size_t rs = fread(filebuffer, sizeof(char), file_size, file);
-if (rs != file_size) {
-fprintf(stderr,"fread read only %lu bytes out of %lu\n",
(long unsigned int) rs, (long unsigned int) file_size);
-}
-filebuffer[file_size]='\0';
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398214258
##
libs/utils/include/celix_properties.h:
##
@@ -17,86 +17,509 @@
* under the License.
*/
+/**
+ * @file celix_properties.h
+ * @brief Header file for the Celix Properties API.
+ *
+ * The Celix Properties API provides a means for storing and manipulating
key-value pairs, called properties,
+ * which can be used to store configuration data or metadata for a service,
component, or framework configuration.
+ * Functions are provided for creating and destroying property sets, loading
and storing properties from/to a file
+ * or stream, and setting, getting, and unsetting individual properties. There
are also functions for converting
+ * property values to various types (e.g. long, bool, double) and for
iterating over the properties in a set.
+ *
+ * Supported property value types include:
+ * - string (char*)
+ * - long
+ * - double
+ * - bool
+ * - celix_version_t*
+ */
+
#ifndef CELIX_PROPERTIES_H_
#define CELIX_PROPERTIES_H_
#include
-#include
#include "celix_cleanup.h"
#include "celix_compiler.h"
#include "celix_errno.h"
#include "celix_utils_export.h"
+#include "celix_version.h"
#ifdef __cplusplus
extern "C" {
#endif
-typedef struct hashMap celix_properties_t; //opaque struct, TODO try to make
this a celix_properties struct
+/**
+ * @brief celix_properties_t is a type that represents a set of key-value
pairs called properties.
+ *
+ * @note Not thread safe.
+ */
+typedef struct celix_properties celix_properties_t;
+
+/**
+ * @brief Enum representing the possible types of a property value.
+ */
+typedef enum celix_properties_value_type {
+CELIX_PROPERTIES_VALUE_TYPE_UNSET = 0, /**< Property value is not set. */
+CELIX_PROPERTIES_VALUE_TYPE_STRING = 1, /**< Property value is a string. */
+CELIX_PROPERTIES_VALUE_TYPE_LONG = 2, /**< Property value is a long
integer. */
+CELIX_PROPERTIES_VALUE_TYPE_DOUBLE = 3, /**< Property value is a double. */
+CELIX_PROPERTIES_VALUE_TYPE_BOOL = 4, /**< Property value is a boolean.
*/
+CELIX_PROPERTIES_VALUE_TYPE_VERSION = 5 /**< Property value is a Celix
version. */
+} celix_properties_value_type_e;
+/**
+ * @brief A structure representing a single value entry in a property set.
+ */
+typedef struct celix_properties_entry {
+const char* value; /**< The string value or string
representation of a non-string
+ typed value.*/
+celix_properties_value_type_e valueType; /**< The type of the value of the
entry */
+
+union {
+const char* strValue;/**< The string value of the
entry. */
+long longValue; /**< The long integer value of
the entry. */
+double doubleValue; /**< The double-precision
floating point value of the entry. */
+bool boolValue; /**< The boolean value of the
entry. */
+const celix_version_t* versionValue; /**< The Celix version value of
the entry. */
+} typed; /**< The typed values of the
entry. Only valid if valueType
+ is not
CELIX_PROPERTIES_VALUE_TYPE_UNSET and only the matching
+ value types should be used.
E.g typed.boolValue if valueType is
+
CELIX_PROPERTIES_VALUE_TYPE_BOOL. */
+} celix_properties_entry_t;
+
+/**
+ * @brief Represents an iterator for iterating over the entries in a
celix_properties_t object.
+ */
typedef struct celix_properties_iterator {
-//private data
-void* _data1;
-void* _data2;
-void* _data3;
-int _data4;
-int _data5;
-} celix_properties_iterator_t;
+/**
+ * @brief The index of the current iterator.
+ */
+size_t index;
+/**
+ * @brief The current key.
+ */
+const char* key;
-/**
-
**
- * Updated API
-
**
-
**/
+/**
+ * @brief The current value entry.
+ */
+celix_properties_entry_t entry;
-CELIX_UTILS_EXPORT celix_properties_t* celix_properties_create(void);
+/**
+ * @brief Private data used to implement the iterator.
+ */
+char _data[56];
+} celix_properties_iterator_t;
-CELIX_UTILS_EXPORT void celix_properties_destroy(celix_properties_t
*properties);
+/**
+ * @brief Create a new empty property set.
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398119159
##
libs/utils/src/properties.c:
##
@@ -103,12 +131,278 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strdup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const
celix_properties_entry_t* prototype) {
+char convertedValueBuffer[21];
+if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
+entry->typed.versionValue = prototype->typed.versionValue;
+bool written = celix_version_fillString(prototype->typed.versionValue,
convertedValueBuffer, sizeof(convertedValueBuffer));
+if (written) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+entry->value =
celix_version_toString(prototype->typed.versionValue);
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG;
+entry->typed.longValue = prototype->typed.longValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%li", entry->typed.longValue);
+if (written >= 0 || written < sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+//LCOV_EXCL_START
+assert(false); // should not happen, LONG_MAX str is 19 chars,
LONG_MIN str is 20 chars
+//LCOV_EXCL_STOP
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE;
+entry->typed.doubleValue = prototype->typed.doubleValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+if (written < 0 || written <= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%f", entry->typed.doubleValue);
+entry->value = val;
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL;
+entry->typed.boolValue = prototype->typed.boolValue;
+entry->value = entry->typed.boolValue ?
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+} else /*string value*/ {
+assert(prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING);
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING;
+entry->value = celix_properties_createString(properties,
prototype->typed.strValue);
+entry->typed.strValue = entry->value;
+}
+
+if (entry->value == NULL) {
+return CELIX_ENOMEM;
+}
+return CELIX_SUCCESS;
+}
+
+/**
+ * Allocate entry and optionally use the short
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398127100
##
libs/utils/src/properties.c:
##
@@ -103,12 +131,265 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strdup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const
celix_properties_entry_t* prototype) {
+char convertedValueBuffer[21] = {0};
+*entry = *prototype;
+if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+bool written = celix_version_fillString(prototype->typed.versionValue,
convertedValueBuffer, sizeof(convertedValueBuffer));
+if (written) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+entry->value =
celix_version_toString(prototype->typed.versionValue);
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) {
+// LONG_MAX str is 19 chars, LONG_MIN str is 20 chars
+(void)snprintf(convertedValueBuffer, sizeof(convertedValueBuffer),
"%li", entry->typed.longValue);
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) {
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+if (written >= 0 || written < sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%f", entry->typed.doubleValue);
+entry->value = val;
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL) {
+entry->value = entry->typed.boolValue ?
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+} else /*string value*/ {
+assert(prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING);
+entry->value = celix_properties_createString(properties,
prototype->typed.strValue);
+entry->typed.strValue = entry->value;
+}
+
+if (entry->value == NULL) {
+return CELIX_ENOMEM;
+}
+return CELIX_SUCCESS;
+}
+
+/**
+ * Allocate entry and optionally use the short properties optimization entries
buffer.
+ */
+celix_properties_entry_t* celix_properties_allocEntry(celix_properties_t*
properties) {
+celix_properties_entry_t* entry;
+if (properties->currentEntriesBufferIndex <
CELIX_PROPERTIES_OPTIMIZATION_ENTRIES_BUFFER_SIZE) {
+entry =
&properties->entriesBuffer[properties->currentEntriesBufferIndex++];
+} else {
+entry = malloc(sizeof(*entry));
+}
+return entry;
+}
+
+/**
+ * Create entry and optionally use the short properties optimization entries
buffer and take ownership of the
+ * provided key and value strings.
+ */
+static celix_properties_entry_t*
celix_properties_createEntryWithNoCopy(celix_properties_t* properties,
+
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398122940
##
libs/utils/src/properties.c:
##
@@ -103,12 +131,265 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strdup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const
celix_properties_entry_t* prototype) {
+char convertedValueBuffer[21] = {0};
+*entry = *prototype;
+if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+bool written = celix_version_fillString(prototype->typed.versionValue,
convertedValueBuffer, sizeof(convertedValueBuffer));
+if (written) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+entry->value =
celix_version_toString(prototype->typed.versionValue);
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) {
+// LONG_MAX str is 19 chars, LONG_MIN str is 20 chars
+(void)snprintf(convertedValueBuffer, sizeof(convertedValueBuffer),
"%li", entry->typed.longValue);
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) {
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+if (written >= 0 || written < sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%f", entry->typed.doubleValue);
+entry->value = val;
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL) {
+entry->value = entry->typed.boolValue ?
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+} else /*string value*/ {
+assert(prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING);
+entry->value = celix_properties_createString(properties,
prototype->typed.strValue);
+entry->typed.strValue = entry->value;
+}
+
+if (entry->value == NULL) {
+return CELIX_ENOMEM;
+}
+return CELIX_SUCCESS;
+}
+
+/**
+ * Allocate entry and optionally use the short properties optimization entries
buffer.
+ */
+celix_properties_entry_t* celix_properties_allocEntry(celix_properties_t*
properties) {
+celix_properties_entry_t* entry;
+if (properties->currentEntriesBufferIndex <
CELIX_PROPERTIES_OPTIMIZATION_ENTRIES_BUFFER_SIZE) {
+entry =
&properties->entriesBuffer[properties->currentEntriesBufferIndex++];
+} else {
+entry = malloc(sizeof(*entry));
+}
+return entry;
+}
+
+/**
+ * Create entry and optionally use the short properties optimization entries
buffer and take ownership of the
+ * provided key and value strings.
+ */
+static celix_properties_entry_t*
celix_properties_createEntryWithNoCopy(celix_properties_t* properties,
+
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398120010
##
libs/utils/src/properties.c:
##
@@ -103,12 +131,265 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strdup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const
celix_properties_entry_t* prototype) {
+char convertedValueBuffer[21] = {0};
+*entry = *prototype;
+if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+bool written = celix_version_fillString(prototype->typed.versionValue,
convertedValueBuffer, sizeof(convertedValueBuffer));
+if (written) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+entry->value =
celix_version_toString(prototype->typed.versionValue);
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) {
+// LONG_MAX str is 19 chars, LONG_MIN str is 20 chars
+(void)snprintf(convertedValueBuffer, sizeof(convertedValueBuffer),
"%li", entry->typed.longValue);
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) {
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+if (written >= 0 || written < sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%f", entry->typed.doubleValue);
+entry->value = val;
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL) {
+entry->value = entry->typed.boolValue ?
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+} else /*string value*/ {
+assert(prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING);
+entry->value = celix_properties_createString(properties,
prototype->typed.strValue);
+entry->typed.strValue = entry->value;
+}
+
+if (entry->value == NULL) {
+return CELIX_ENOMEM;
+}
+return CELIX_SUCCESS;
+}
+
+/**
+ * Allocate entry and optionally use the short properties optimization entries
buffer.
+ */
+celix_properties_entry_t* celix_properties_allocEntry(celix_properties_t*
properties) {
+celix_properties_entry_t* entry;
+if (properties->currentEntriesBufferIndex <
CELIX_PROPERTIES_OPTIMIZATION_ENTRIES_BUFFER_SIZE) {
+entry =
&properties->entriesBuffer[properties->currentEntriesBufferIndex++];
+} else {
+entry = malloc(sizeof(*entry));
+}
+return entry;
+}
+
+/**
+ * Create entry and optionally use the short properties optimization entries
buffer and take ownership of the
+ * provided key and value strings.
+ */
+static celix_properties_entry_t*
celix_properties_createEntryWithNoCopy(celix_properties_t* properties,
+
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398119159
##
libs/utils/src/properties.c:
##
@@ -103,12 +131,278 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strdup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const
celix_properties_entry_t* prototype) {
+char convertedValueBuffer[21];
+if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
+entry->typed.versionValue = prototype->typed.versionValue;
+bool written = celix_version_fillString(prototype->typed.versionValue,
convertedValueBuffer, sizeof(convertedValueBuffer));
+if (written) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+entry->value =
celix_version_toString(prototype->typed.versionValue);
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG;
+entry->typed.longValue = prototype->typed.longValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%li", entry->typed.longValue);
+if (written >= 0 || written < sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+//LCOV_EXCL_START
+assert(false); // should not happen, LONG_MAX str is 19 chars,
LONG_MIN str is 20 chars
+//LCOV_EXCL_STOP
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE;
+entry->typed.doubleValue = prototype->typed.doubleValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+if (written < 0 || written <= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%f", entry->typed.doubleValue);
+entry->value = val;
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL;
+entry->typed.boolValue = prototype->typed.boolValue;
+entry->value = entry->typed.boolValue ?
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+} else /*string value*/ {
+assert(prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING);
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING;
+entry->value = celix_properties_createString(properties,
prototype->typed.strValue);
+entry->typed.strValue = entry->value;
+}
+
+if (entry->value == NULL) {
+return CELIX_ENOMEM;
+}
+return CELIX_SUCCESS;
+}
+
+/**
+ * Allocate entry and optionally use the short
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398119159
##
libs/utils/src/properties.c:
##
@@ -103,12 +131,278 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strdup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const
celix_properties_entry_t* prototype) {
+char convertedValueBuffer[21];
+if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
+entry->typed.versionValue = prototype->typed.versionValue;
+bool written = celix_version_fillString(prototype->typed.versionValue,
convertedValueBuffer, sizeof(convertedValueBuffer));
+if (written) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+entry->value =
celix_version_toString(prototype->typed.versionValue);
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG;
+entry->typed.longValue = prototype->typed.longValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%li", entry->typed.longValue);
+if (written >= 0 || written < sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+//LCOV_EXCL_START
+assert(false); // should not happen, LONG_MAX str is 19 chars,
LONG_MIN str is 20 chars
+//LCOV_EXCL_STOP
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE;
+entry->typed.doubleValue = prototype->typed.doubleValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+if (written < 0 || written <= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%f", entry->typed.doubleValue);
+entry->value = val;
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL;
+entry->typed.boolValue = prototype->typed.boolValue;
+entry->value = entry->typed.boolValue ?
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+} else /*string value*/ {
+assert(prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING);
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING;
+entry->value = celix_properties_createString(properties,
prototype->typed.strValue);
+entry->typed.strValue = entry->value;
+}
+
+if (entry->value == NULL) {
+return CELIX_ENOMEM;
+}
+return CELIX_SUCCESS;
+}
+
+/**
+ * Allocate entry and optionally use the short
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398088103
##
libs/utils/src/properties.c:
##
@@ -103,12 +131,278 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strdup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const
celix_properties_entry_t* prototype) {
+char convertedValueBuffer[21];
+if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
+entry->typed.versionValue = prototype->typed.versionValue;
+bool written = celix_version_fillString(prototype->typed.versionValue,
convertedValueBuffer, sizeof(convertedValueBuffer));
+if (written) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+entry->value =
celix_version_toString(prototype->typed.versionValue);
+}
+} else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG;
+entry->typed.longValue = prototype->typed.longValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%li", entry->typed.longValue);
+if (written < 0 || written <= sizeof(convertedValueBuffer)) {
Review Comment:
```suggestion
if (written >= 0 || written < sizeof(convertedValueBuffer)) {
```
`written < 0 || written == sizeof(convertedValueBuffer)` should be error.
According to `man snprintf`, " a return value of size or more means that the
output was truncated".
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398054410
##
libs/utils/src/properties.c:
##
@@ -103,12 +131,278 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strdup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const
celix_properties_entry_t* prototype) {
+char convertedValueBuffer[20];
Review Comment:
```suggestion
char convertedValueBuffer[21];
```
One extra byte for the null character.
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1397781840
##
libs/utils/src/properties.c:
##
@@ -103,12 +132,275 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
Review Comment:
I think this is not true. The reason is that the celix_properties_freeString
can be called because of a `celix_properties_unser` and in that case the to be
"freed" string is not the last entry in string buffer.
Of course it is possible (with the pointer and len value) to move the rest
of the string value earlier in the buffer, but IMO this adds to much complexity
for these corner cases of fragmented properties optimized string buffer (ENOMEM
or unset).
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1397781840
##
libs/utils/src/properties.c:
##
@@ -103,12 +132,275 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
Review Comment:
I think this is not true. The reason is that the celix_properties_freeString
can be called because of a `celix_properties_unser` and in that case the freed
string is not the last entry in string buffer.
Of course it is possible (with the pointer and len value) to move the rest
of the string value earlier in the buffer, but IMO this adds to much complexity
for these corner cases of fragmented properties optimized string buffer (ENOMEM
or unset).
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r139996
##
libs/utils/src/properties.c:
##
@@ -103,12 +132,275 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const char** strValue,
+ const long* longValue,
+ const double* doubleValue,
+ const bool* boolValue,
+ celix_version_t*
versionValue) {
+char convertedValueBuffer[32];
+if (strValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING;
+entry->value = celix_properties_createString(properties, *strValue);
+entry->typed.strValue = entry->value;
+} else if (longValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG;
+entry->typed.longValue = *longValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%li", entry->typed.longValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%li", entry->typed.longValue);
+entry->value = val;
+}
+} else if (doubleValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE;
+entry->typed.doubleValue = *doubleValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%f", entry->typed.doubleValue);
+entry->value = val;
+}
+} else if (boolValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL;
+entry->typed.boolValue = *boolValue;
+entry->value = entry->typed.boolValue ?
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+} else /*versionValue*/ {
+assert(versionValue != NULL);
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
+entry->typed.versionValue = versionValue;
+bool written = celix_version_fillString(versionValue,
convertedValueBuffer, sizeof(convertedValueBuffer));
+if (written) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+entry->value = celix_version_toString(versionValue);
+}
+}
+if (entry->value == NULL) {
+return CELIX_ENOMEM;
+}
+return CELIX_SUCCESS;
+}
+
+/**
+ * Allocate entry and optionally use the short properties optimization entries
buffer.
+ */
+celix_propertie
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1397775794
##
libs/utils/src/properties.c:
##
@@ -103,12 +132,275 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const char** strValue,
+ const long* longValue,
+ const double* doubleValue,
+ const bool* boolValue,
+ celix_version_t*
versionValue) {
+char convertedValueBuffer[32];
+if (strValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING;
+entry->value = celix_properties_createString(properties, *strValue);
+entry->typed.strValue = entry->value;
+} else if (longValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG;
+entry->typed.longValue = *longValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%li", entry->typed.longValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%li", entry->typed.longValue);
+entry->value = val;
+}
+} else if (doubleValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE;
+entry->typed.doubleValue = *doubleValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%f", entry->typed.doubleValue);
+entry->value = val;
+}
+} else if (boolValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL;
+entry->typed.boolValue = *boolValue;
+entry->value = entry->typed.boolValue ?
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+} else /*versionValue*/ {
+assert(versionValue != NULL);
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
+entry->typed.versionValue = versionValue;
+bool written = celix_version_fillString(versionValue,
convertedValueBuffer, sizeof(convertedValueBuffer));
+if (written) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+entry->value = celix_version_toString(versionValue);
+}
+}
+if (entry->value == NULL) {
+return CELIX_ENOMEM;
+}
+return CELIX_SUCCESS;
+}
+
+/**
+ * Allocate entry and optionally use the short properties optimization entries
buffer.
+ */
+celix_propertie
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1397750197
##
libs/utils/src/properties.c:
##
@@ -103,12 +132,275 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const char** strValue,
+ const long* longValue,
+ const double* doubleValue,
+ const bool* boolValue,
+ celix_version_t*
versionValue) {
+char convertedValueBuffer[32];
+if (strValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING;
+entry->value = celix_properties_createString(properties, *strValue);
+entry->typed.strValue = entry->value;
+} else if (longValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG;
+entry->typed.longValue = *longValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%li", entry->typed.longValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%li", entry->typed.longValue);
+entry->value = val;
+}
+} else if (doubleValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE;
+entry->typed.doubleValue = *doubleValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%f", entry->typed.doubleValue);
+entry->value = val;
+}
+} else if (boolValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL;
+entry->typed.boolValue = *boolValue;
+entry->value = entry->typed.boolValue ?
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+} else /*versionValue*/ {
+assert(versionValue != NULL);
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
+entry->typed.versionValue = versionValue;
+bool written = celix_version_fillString(versionValue,
convertedValueBuffer, sizeof(convertedValueBuffer));
+if (written) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+entry->value = celix_version_toString(versionValue);
+}
+}
+if (entry->value == NULL) {
+return CELIX_ENOMEM;
+}
+return CELIX_SUCCESS;
+}
+
+/**
+ * Allocate entry and optionally use the short properties optimization entries
buffer.
+ */
+celix_propertie
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1391103653
##
libs/utils/src/celix_hash_map.c:
##
@@ -599,33 +675,49 @@ bool celix_longHashMapIterator_isEnd(const
celix_long_hash_map_iterator_t* iter)
void celix_stringHashMapIterator_next(celix_string_hash_map_iterator_t* iter) {
const celix_hash_map_t* map = iter->_internal[0];
celix_hash_map_entry_t *entry = iter->_internal[1];
+iter->index += 1;
Review Comment:
I agree that "the index of the end iterator is the same as the size of the
map".
The current implementation of `celix_stringHashMapIterator_next` allows
`iter->index > map->genericMap.size`. Is it intended?
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1391120936
##
libs/utils/src/properties.c:
##
@@ -191,324 +476,423 @@ static void parseLine(const char* line,
celix_properties_t *props) {
}
if (!isComment) {
-//printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
+// printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
celix_properties_set(props, celix_utils_trimInPlace(key),
celix_utils_trimInPlace(value));
Review Comment:
Maybe we delay it to when solving #685.
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1391103653
##
libs/utils/src/celix_hash_map.c:
##
@@ -599,33 +675,49 @@ bool celix_longHashMapIterator_isEnd(const
celix_long_hash_map_iterator_t* iter)
void celix_stringHashMapIterator_next(celix_string_hash_map_iterator_t* iter) {
const celix_hash_map_t* map = iter->_internal[0];
celix_hash_map_entry_t *entry = iter->_internal[1];
+iter->index += 1;
Review Comment:
I agree that "the index of the end iterator is the same as the size of the
map".
The current implementation of `celix_stringHashMapIterator_next` allow
`iter->index > map->genericMap.size`. Is it intended?
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390973000
##
libs/utils/src/celix_hash_map.c:
##
@@ -19,64 +19,56 @@
#include "celix_string_hash_map.h"
#include "celix_long_hash_map.h"
-#include "celix_utils.h"
+#include "celix_hash_map_private.h"
+#include "celix_hash_map_internal.h"
#include
#include
#include
#include
#include
-/**
- * Whether to use realloc - instead of calloc - to resize a hash map.
- *
- * With realloc the memory is increased and the
- * map entries are corrected.
- *
- * With alloc a new buckets array is allocated and
- * entries are moved into the new bucket array.
- * And the old buckets array is freed.
- *
- */
-#define CELIX_HASH_MAP_RESIZE_WITH_REALLOC
-
-static unsigned int DEFAULT_INITIAL_CAPACITY = 16;
-static double DEFAULT_LOAD_FACTOR = 0.75;
-static unsigned int MAXIMUM_CAPACITY = INT32_MAX/10;
+#include "celix_utils.h"
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
-typedef enum celix_hash_map_key_type {
-CELIX_HASH_MAP_STRING_KEY,
-CELIX_HASH_MAP_LONG_KEY
-} celix_hash_map_key_type_e;
+#define CELIX_HASHMAP_DEFAULT_INITIAL_CAPACITY 16
+#define CELIX_HASHMAP_DEFAULT_MAX_LOAD_FACTOR 5.0
+#define CELIX_HASHMAP_CAPACITY_INCREASE_FACTOR 2
+#define CELIX_HASHMAP_MAXIMUM_CAPACITY (INT32_MAX/10)
+#define CELIX_HASHMAP_MAXIMUM_INCREASE_VALUE (1024*10)
+#define CELIX_HASHMAP_HASH_PRIME 1610612741
-typedef union celix_hash_map_key {
+union celix_hash_map_key {
const char* strKey;
long longKey;
-} celix_hash_map_key_t;
+};
-typedef struct celix_hash_map_entry celix_hash_map_entry_t;
struct celix_hash_map_entry {
celix_hash_map_key_t key;
celix_hash_map_value_t value;
celix_hash_map_entry_t* next;
unsigned int hash;
};
-typedef struct celix_hash_map {
+struct celix_hash_map {
celix_hash_map_entry_t** buckets;
unsigned int bucketsSize; //nr of buckets
unsigned int size; //nr of total entries
-double loadFactor;
+double maxLoadFactor;
celix_hash_map_key_type_e keyType;
-celix_hash_map_value_t emptyValue;
-unsigned int (*hashKeyFunction)(const celix_hash_map_key_t* key);
-bool (*equalsKeyFunction)(const celix_hash_map_key_t* key1, const
celix_hash_map_key_t* key2);
void (*simpleRemovedCallback)(void* value);
void* removedCallbackData;
-void (*removedStringKeyCallback)(void* data, const char* removedKey,
celix_hash_map_value_t removedValue);
-void (*removedLongKeyCallback)(void* data, long removedKey,
celix_hash_map_value_t removedValue);
+void (*removedStringEntryCallback)(void* data, const char* removedKey,
celix_hash_map_value_t removedValue);
+void (*removedStringKeyCallback)(void* data, char* key);
+void (*removedLongEntryCallback)(void* data, long removedKey,
celix_hash_map_value_t removedValue);
bool storeKeysWeakly;
-} celix_hash_map_t;
+
+//statistics
+size_t modificationsCount;
+size_t resizeCount;
+size_t collisionsCount;
Review Comment:
`modificationsCount` and `collisionsCount` are unused now.
##
libs/utils/include/celix_long_hash_map.h:
##
@@ -105,21 +114,21 @@ typedef struct celix_long_hash_map_create_options {
unsigned int initialCapacity CELIX_OPTS_INIT;
/**
- * @brief The hash map load factor, which controls the max ratio between
nr of entries in the hash map and the
+ * @brief The hash map max load factor, which controls the max ratio
between nr of entries in the hash map and the
* hash map capacity.
*
- * The load factor controls how large the hash map capacity (nr of
buckets) is compared to the nr of entries
+ * The max load factor controls how large the hash map capacity (nr of
buckets) is compared to the nr of entries
* in the hash map. The load factor is an important property of the hash
map which influences how close the
* hash map performs to O(1) for its get, has and put operations.
*
- * If the nr of entries increases above the loadFactor * capacity, the
hash capacity will be doubled.
+ * If the nr of entries increases above the maxLoadFactor * capacity, the
hash table capacity will be doubled.
* For example a hash map with capacity 16 and load factor 0.75 will
double its capacity when the 13th entry
* is added to the hash map.
*
* If 0 is provided, the hash map load factor will be 0.75 (default hash
map load factor).
Review Comment:
```suggestion
* If 0 is provided, the hash map load factor will be 5 (default hash
map load factor).
```
##
libs/utils/src/celix_hash_map.c:
##
@@ -599,33 +675,49 @@ bool celix_longHashMapIterator_isEnd(const
celix_long_hash_map_iterator_t* iter)
void celix_stringHashMapIterator_next(celix_string_hash_map_iterator_t* iter) {
const celix_hash_map_t* map = iter->_internal[0];
celix_hash_map_entry_t *entry = iter->_internal[1];
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390458663
##
libs/utils/src/celix_hash_map.c:
##
@@ -94,11 +83,11 @@ static unsigned int celix_longHashMap_hash(const
celix_hash_map_key_t* key) {
return key->longKey ^ (key->longKey >> (sizeof(key->longKey)*8/2));
Review Comment:
I think this has been addressed by trying out and selecting a different long
hash function.
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390458391
##
libs/utils/src/celix_hash_map.c:
##
@@ -339,21 +327,27 @@ static bool celix_hashMap_remove(celix_hash_map_t* map,
const char* strKey, long
visit = visit->next;
}
if (removedEntry != NULL) {
+char* removedKey = NULL;
+if (map->keyType == CELIX_HASH_MAP_STRING_KEY) {
+removedKey = (char*)removedEntry->key.strKey;
+}
celix_hashMap_destroyRemovedEntry(map, removedEntry);
+if (removedKey) {
+celix_hashMap_destroyRemovedKey(map, removedKey);
+}
return true;
}
return false;
}
-static void celix_hashMap_init(
+celix_status_t celix_hashMap_init(
celix_hash_map_t* map,
celix_hash_map_key_type_e keyType,
unsigned int initialCapacity,
double loadFactor,
unsigned int (*hashKeyFn)(const celix_hash_map_key_t*),
bool (*equalsKeyFn)(const celix_hash_map_key_t*, const
celix_hash_map_key_t*)) {
map->loadFactor = loadFactor;
Review Comment:
Here are the hash function benchmark results (on my machine, using a release
build type and cpupower on performance):
hash1 is the long hash function used in celix_long_hash_map at the start of
this PR
hash2 is the long hash function used in the deprecated hashmap
hash3 is a altered long hash function which uses a prime.
I also had a short look into
[murmur3](https://github.com/logrhythm/MurMurHash/blob/master/src/MurMurHash.cpp)
and [FNV](https://github.com/fnvhash/libfnv/tree/master) hash, but for the
moment I think this requires too much work (technical and license wise)
# Hash1, max load 5
```
-
Benchmark
Time CPU Iterations UserCounters...
-
LongHashmapBenchmark_findEntryFromStdMap/10/process_time/real_time
0.432 ns0.432 ns 10 items_per_second=2.31246G/s
LongHashmapBenchmark_findEntryFromStdMap/100/process_time/real_time
0.433 ns0.433 ns 10 items_per_second=2.31041G/s
LongHashmapBenchmark_findEntryFromStdMap/1000/process_time/real_time
0.433 ns0.433 ns 10 items_per_second=2.31151G/s
LongHashmapBenchmark_findEntryFromStdMap/1/process_time/real_time
0.434 ns0.434 ns 10 items_per_second=2.30551G/s
LongHashmapBenchmark_findEntryFromStdMap/10/process_time/real_time
0.433 ns0.433 ns 10 items_per_second=2.31037G/s
LongHashmapBenchmark_findEntryFromStdMap/100/process_time/real_time
0.433 ns0.432 ns 10 items_per_second=2.30713G/s
LongHashmapBenchmark_findEntryFromCelixMap/10/process_time/real_time
2.38 ns 2.38 ns293482551 averageNrOfEntriesPerBucket=0.625
items_per_second=420.077M/s nrOfBuckets=16 resizeCount=0
stdDeviationNrOfEntriesPerBucket=0.856957
LongHashmapBenchmark_findEntryFromCelixMap/100/process_time/real_time
2.64 ns 2.63 ns268351618 averageNrOfEntriesPerBucket=3.125
items_per_second=379.474M/s nrOfBuckets=32 resizeCount=1
stdDeviationNrOfEntriesPerBucket=1.91621
LongHashmapBenchmark_findEntryFromCelixMap/1000/process_time/real_time
2.16 ns 2.16 ns323130939
averageNrOfEntriesPerBucket=3.90625 items_per_second=462.94M/s nrOfBuckets=256
resizeCount=4 stdDeviationNrOfEntriesPerBucket=1.80467
LongHashmapBenchmark_findEntryFromCelixMap/1/process_time/real_time
3.90 ns 3.89 ns183397882
averageNrOfEntriesPerBucket=4.88281 items_per_second=256.477M/s
nrOfBuckets=2.048k resizeCount=7 stdDeviationNrOfEntriesPerBucket=2.20062
LongHashmapBenchmark_findEntryFromCelixMap/10/process_time/real_time
2.38 ns 2.38 ns294279918
averageNrOfEntriesPerBucket=3.75601 items_per_second=420.113M/s
nrOfBuckets=26.624k resizeCount=11 stdDeviationNrOfEntriesPerBucket=1.93968
LongHashmapBenchmark_findEntryFromCelixMap/100/process_time/real_time
2.62 ns 2.62 ns270684694
averageNrOfEntriesPerBucket=4.98246 items_per_second=380.976M/s
nrOfBuckets=200.704k resizeCount=28 stdDeviationNrOfEntriesPerBucket=2.23419
LongHashmapBenchmark_findEntryFromDeprecatedMap/10/process_time/real_time
3.67 ns 3.67 ns190401851 items_per_second=272.19M/s
LongHashmapBenchmark_findEntryFromDeprecatedMap/100/process_time/real_time
3.69 ns 3.68 ns190481474 i
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390457651
##
libs/utils/src/celix_hash_map.c:
##
@@ -339,21 +327,27 @@ static bool celix_hashMap_remove(celix_hash_map_t* map,
const char* strKey, long
visit = visit->next;
}
if (removedEntry != NULL) {
+char* removedKey = NULL;
+if (map->keyType == CELIX_HASH_MAP_STRING_KEY) {
+removedKey = (char*)removedEntry->key.strKey;
+}
celix_hashMap_destroyRemovedEntry(map, removedEntry);
+if (removedKey) {
+celix_hashMap_destroyRemovedKey(map, removedKey);
+}
return true;
}
return false;
}
-static void celix_hashMap_init(
+celix_status_t celix_hashMap_init(
celix_hash_map_t* map,
celix_hash_map_key_type_e keyType,
unsigned int initialCapacity,
double loadFactor,
unsigned int (*hashKeyFn)(const celix_hash_map_key_t*),
bool (*equalsKeyFn)(const celix_hash_map_key_t*, const
celix_hash_map_key_t*)) {
map->loadFactor = loadFactor;
Review Comment:
Here are the load factor benchmark results (on my machine, using a release
build type and cpupower on performance):
# Hash3 , max load 0.75
```
-
Benchmark
Time CPU Iterations UserCounters...
-
LongHashmapBenchmark_findEntryFromStdMap/10/process_time/real_time
0.421 ns0.421 ns 10 items_per_second=2.37362G/s
LongHashmapBenchmark_findEntryFromStdMap/100/process_time/real_time
0.421 ns0.421 ns 10 items_per_second=2.37341G/s
LongHashmapBenchmark_findEntryFromStdMap/1000/process_time/real_time
0.422 ns0.422 ns 10 items_per_second=2.36961G/s
LongHashmapBenchmark_findEntryFromStdMap/1/process_time/real_time
0.421 ns0.421 ns 10 items_per_second=2.37266G/s
LongHashmapBenchmark_findEntryFromStdMap/10/process_time/real_time
0.422 ns0.422 ns 10 items_per_second=2.3715G/s
LongHashmapBenchmark_findEntryFromStdMap/100/process_time/real_time
0.422 ns0.422 ns 10 items_per_second=2.3694G/s
LongHashmapBenchmark_findEntryFromCelixMap/10/process_time/real_time
2.32 ns 2.32 ns302202596 averageNrOfEntriesPerBucket=0.625
items_per_second=431.121M/s nrOfBuckets=16 resizeCount=0
stdDeviationNrOfEntriesPerBucket=0.856957
LongHashmapBenchmark_findEntryFromCelixMap/100/process_time/real_time
2.58 ns 2.58 ns272789300
averageNrOfEntriesPerBucket=0.390625 items_per_second=387.522M/s
nrOfBuckets=256 resizeCount=4 stdDeviationNrOfEntriesPerBucket=0.687322
LongHashmapBenchmark_findEntryFromCelixMap/1000/process_time/real_time
2.37 ns 2.37 ns295845710
averageNrOfEntriesPerBucket=0.488281 items_per_second=421.411M/s
nrOfBuckets=2.048k resizeCount=7 stdDeviationNrOfEntriesPerBucket=0.695872
LongHashmapBenchmark_findEntryFromCelixMap/1/process_time/real_time
2.39 ns 2.39 ns294741774
averageNrOfEntriesPerBucket=0.610352 items_per_second=417.892M/s
nrOfBuckets=16.384k resizeCount=10 stdDeviationNrOfEntriesPerBucket=0.784076
LongHashmapBenchmark_findEntryFromCelixMap/10/process_time/real_time
2.40 ns 2.40 ns297425736
averageNrOfEntriesPerBucket=0.718061 items_per_second=417.292M/s
nrOfBuckets=139.264k resizeCount=22 stdDeviationNrOfEntriesPerBucket=0.846435
LongHashmapBenchmark_findEntryFromCelixMap/100/process_time/real_time
2.27 ns 2.27 ns308634832
averageNrOfEntriesPerBucket=0.747751 items_per_second=440.035M/s
nrOfBuckets=1.33734M resizeCount=139 stdDeviationNrOfEntriesPerBucket=0.863872
LongHashmapBenchmark_findEntryFromDeprecatedMap/10/process_time/real_time
3.53 ns 3.53 ns198927031 items_per_second=283.682M/s
LongHashmapBenchmark_findEntryFromDeprecatedMap/100/process_time/real_time
3.52 ns 3.52 ns198043432 items_per_second=283.711M/s
LongHashmapBenchmark_findEntryFromDeprecatedMap/1000/process_time/real_time
3.53 ns 3.53 ns195734925 items_per_second=283.213M/s
LongHashmapBenchmark_findEntryFromDeprecatedMap/1/process_time/real_time
3.53 ns 3.53 ns197803895 items_per_second=283.436M/s
LongHashmapBenchmark_findEntryFromDeprecatedMap/10/process_time/real_time
3.52 ns 3.52 ns198613048 item
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390458391
##
libs/utils/src/celix_hash_map.c:
##
@@ -339,21 +327,27 @@ static bool celix_hashMap_remove(celix_hash_map_t* map,
const char* strKey, long
visit = visit->next;
}
if (removedEntry != NULL) {
+char* removedKey = NULL;
+if (map->keyType == CELIX_HASH_MAP_STRING_KEY) {
+removedKey = (char*)removedEntry->key.strKey;
+}
celix_hashMap_destroyRemovedEntry(map, removedEntry);
+if (removedKey) {
+celix_hashMap_destroyRemovedKey(map, removedKey);
+}
return true;
}
return false;
}
-static void celix_hashMap_init(
+celix_status_t celix_hashMap_init(
celix_hash_map_t* map,
celix_hash_map_key_type_e keyType,
unsigned int initialCapacity,
double loadFactor,
unsigned int (*hashKeyFn)(const celix_hash_map_key_t*),
bool (*equalsKeyFn)(const celix_hash_map_key_t*, const
celix_hash_map_key_t*)) {
map->loadFactor = loadFactor;
Review Comment:
Here are the hash function benchmark results (on my machine, using a release
build type and cpupower on performance):
hash1 is the long hash function used in celix_long_hash_map at the start of
this PR
hash2 is the long hash function used in the deprecated hashmap
hash3 is a altered long hash function which uses a prime.
I also had a short look into
[murmur3](https://github.com/logrhythm/MurMurHash/blob/master/src/MurMurHash.cpp)
and [FNV](https://github.com/fnvhash/libfnv/tree/master) hash, but for the
moment I think this requires too much work (technical and license wise)
# Hash1, max load 5
```
-
Benchmark
Time CPU Iterations UserCounters...
-
LongHashmapBenchmark_findEntryFromStdMap/10/process_time/real_time
0.432 ns0.432 ns 10 items_per_second=2.31246G/s
LongHashmapBenchmark_findEntryFromStdMap/100/process_time/real_time
0.433 ns0.433 ns 10 items_per_second=2.31041G/s
LongHashmapBenchmark_findEntryFromStdMap/1000/process_time/real_time
0.433 ns0.433 ns 10 items_per_second=2.31151G/s
LongHashmapBenchmark_findEntryFromStdMap/1/process_time/real_time
0.434 ns0.434 ns 10 items_per_second=2.30551G/s
LongHashmapBenchmark_findEntryFromStdMap/10/process_time/real_time
0.433 ns0.433 ns 10 items_per_second=2.31037G/s
LongHashmapBenchmark_findEntryFromStdMap/100/process_time/real_time
0.433 ns0.432 ns 10 items_per_second=2.30713G/s
LongHashmapBenchmark_findEntryFromCelixMap/10/process_time/real_time
2.38 ns 2.38 ns293482551 averageNrOfEntriesPerBucket=0.625
items_per_second=420.077M/s nrOfBuckets=16 resizeCount=0
stdDeviationNrOfEntriesPerBucket=0.856957
LongHashmapBenchmark_findEntryFromCelixMap/100/process_time/real_time
2.64 ns 2.63 ns268351618 averageNrOfEntriesPerBucket=3.125
items_per_second=379.474M/s nrOfBuckets=32 resizeCount=1
stdDeviationNrOfEntriesPerBucket=1.91621
LongHashmapBenchmark_findEntryFromCelixMap/1000/process_time/real_time
2.16 ns 2.16 ns323130939
averageNrOfEntriesPerBucket=3.90625 items_per_second=462.94M/s nrOfBuckets=256
resizeCount=4 stdDeviationNrOfEntriesPerBucket=1.80467
LongHashmapBenchmark_findEntryFromCelixMap/1/process_time/real_time
3.90 ns 3.89 ns183397882
averageNrOfEntriesPerBucket=4.88281 items_per_second=256.477M/s
nrOfBuckets=2.048k resizeCount=7 stdDeviationNrOfEntriesPerBucket=2.20062
LongHashmapBenchmark_findEntryFromCelixMap/10/process_time/real_time
2.38 ns 2.38 ns294279918
averageNrOfEntriesPerBucket=3.75601 items_per_second=420.113M/s
nrOfBuckets=26.624k resizeCount=11 stdDeviationNrOfEntriesPerBucket=1.93968
LongHashmapBenchmark_findEntryFromCelixMap/100/process_time/real_time
2.62 ns 2.62 ns270684694
averageNrOfEntriesPerBucket=4.98246 items_per_second=380.976M/s
nrOfBuckets=200.704k resizeCount=28 stdDeviationNrOfEntriesPerBucket=2.23419
LongHashmapBenchmark_findEntryFromDeprecatedMap/10/process_time/real_time
3.67 ns 3.67 ns190401851 items_per_second=272.19M/s
LongHashmapBenchmark_findEntryFromDeprecatedMap/100/process_time/real_time
3.69 ns 3.68 ns190481474 i
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390457651
##
libs/utils/src/celix_hash_map.c:
##
@@ -339,21 +327,27 @@ static bool celix_hashMap_remove(celix_hash_map_t* map,
const char* strKey, long
visit = visit->next;
}
if (removedEntry != NULL) {
+char* removedKey = NULL;
+if (map->keyType == CELIX_HASH_MAP_STRING_KEY) {
+removedKey = (char*)removedEntry->key.strKey;
+}
celix_hashMap_destroyRemovedEntry(map, removedEntry);
+if (removedKey) {
+celix_hashMap_destroyRemovedKey(map, removedKey);
+}
return true;
}
return false;
}
-static void celix_hashMap_init(
+celix_status_t celix_hashMap_init(
celix_hash_map_t* map,
celix_hash_map_key_type_e keyType,
unsigned int initialCapacity,
double loadFactor,
unsigned int (*hashKeyFn)(const celix_hash_map_key_t*),
bool (*equalsKeyFn)(const celix_hash_map_key_t*, const
celix_hash_map_key_t*)) {
map->loadFactor = loadFactor;
Review Comment:
Here are the load factor benchmark result (on my machine, using a release
build type and cpupower on performance):
# Hash3 , max load 0.75
```
-
Benchmark
Time CPU Iterations UserCounters...
-
LongHashmapBenchmark_findEntryFromStdMap/10/process_time/real_time
0.421 ns0.421 ns 10 items_per_second=2.37362G/s
LongHashmapBenchmark_findEntryFromStdMap/100/process_time/real_time
0.421 ns0.421 ns 10 items_per_second=2.37341G/s
LongHashmapBenchmark_findEntryFromStdMap/1000/process_time/real_time
0.422 ns0.422 ns 10 items_per_second=2.36961G/s
LongHashmapBenchmark_findEntryFromStdMap/1/process_time/real_time
0.421 ns0.421 ns 10 items_per_second=2.37266G/s
LongHashmapBenchmark_findEntryFromStdMap/10/process_time/real_time
0.422 ns0.422 ns 10 items_per_second=2.3715G/s
LongHashmapBenchmark_findEntryFromStdMap/100/process_time/real_time
0.422 ns0.422 ns 10 items_per_second=2.3694G/s
LongHashmapBenchmark_findEntryFromCelixMap/10/process_time/real_time
2.32 ns 2.32 ns302202596 averageNrOfEntriesPerBucket=0.625
items_per_second=431.121M/s nrOfBuckets=16 resizeCount=0
stdDeviationNrOfEntriesPerBucket=0.856957
LongHashmapBenchmark_findEntryFromCelixMap/100/process_time/real_time
2.58 ns 2.58 ns272789300
averageNrOfEntriesPerBucket=0.390625 items_per_second=387.522M/s
nrOfBuckets=256 resizeCount=4 stdDeviationNrOfEntriesPerBucket=0.687322
LongHashmapBenchmark_findEntryFromCelixMap/1000/process_time/real_time
2.37 ns 2.37 ns295845710
averageNrOfEntriesPerBucket=0.488281 items_per_second=421.411M/s
nrOfBuckets=2.048k resizeCount=7 stdDeviationNrOfEntriesPerBucket=0.695872
LongHashmapBenchmark_findEntryFromCelixMap/1/process_time/real_time
2.39 ns 2.39 ns294741774
averageNrOfEntriesPerBucket=0.610352 items_per_second=417.892M/s
nrOfBuckets=16.384k resizeCount=10 stdDeviationNrOfEntriesPerBucket=0.784076
LongHashmapBenchmark_findEntryFromCelixMap/10/process_time/real_time
2.40 ns 2.40 ns297425736
averageNrOfEntriesPerBucket=0.718061 items_per_second=417.292M/s
nrOfBuckets=139.264k resizeCount=22 stdDeviationNrOfEntriesPerBucket=0.846435
LongHashmapBenchmark_findEntryFromCelixMap/100/process_time/real_time
2.27 ns 2.27 ns308634832
averageNrOfEntriesPerBucket=0.747751 items_per_second=440.035M/s
nrOfBuckets=1.33734M resizeCount=139 stdDeviationNrOfEntriesPerBucket=0.863872
LongHashmapBenchmark_findEntryFromDeprecatedMap/10/process_time/real_time
3.53 ns 3.53 ns198927031 items_per_second=283.682M/s
LongHashmapBenchmark_findEntryFromDeprecatedMap/100/process_time/real_time
3.52 ns 3.52 ns198043432 items_per_second=283.711M/s
LongHashmapBenchmark_findEntryFromDeprecatedMap/1000/process_time/real_time
3.53 ns 3.53 ns195734925 items_per_second=283.213M/s
LongHashmapBenchmark_findEntryFromDeprecatedMap/1/process_time/real_time
3.53 ns 3.53 ns197803895 items_per_second=283.436M/s
LongHashmapBenchmark_findEntryFromDeprecatedMap/10/process_time/real_time
3.52 ns 3.52 ns198613048 items
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390457541
##
libs/utils/src/celix_hash_map.c:
##
@@ -339,21 +327,27 @@ static bool celix_hashMap_remove(celix_hash_map_t* map,
const char* strKey, long
visit = visit->next;
}
if (removedEntry != NULL) {
+char* removedKey = NULL;
+if (map->keyType == CELIX_HASH_MAP_STRING_KEY) {
+removedKey = (char*)removedEntry->key.strKey;
+}
celix_hashMap_destroyRemovedEntry(map, removedEntry);
+if (removedKey) {
+celix_hashMap_destroyRemovedKey(map, removedKey);
+}
return true;
}
return false;
}
-static void celix_hashMap_init(
+celix_status_t celix_hashMap_init(
celix_hash_map_t* map,
celix_hash_map_key_type_e keyType,
unsigned int initialCapacity,
double loadFactor,
unsigned int (*hashKeyFn)(const celix_hash_map_key_t*),
bool (*equalsKeyFn)(const celix_hash_map_key_t*, const
celix_hash_map_key_t*)) {
map->loadFactor = loadFactor;
Review Comment:
Interesting. The load factor of 0.75 was and still is used in the deprecated
hash map. I did reading up on hash maps when I created the long/string hash
map, but apparently missed this and reused the 0.75 factor.
I spent some hours tweaking the max load factor and hash functions combined
with the already existing hash map benchmarks. Based on that result I changed
the default long hash, refactored some code to prevent some function calls and
changed the max load factor.
Max load factor is now 5.0 and the long hash function is also changed (uses
a prime number for an overall better distribution in the hash map).
I quite certain more improvements are possible for the long hash map and
this can also been seen in the benchmark reference (`std::unordered_map with a
long key`).
The celix string hash map performance, for finding an entry in the hash map,
is quite close to the std::unordered_map with a string key.
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390439644
##
libs/utils/src/celix_hash_map.c:
##
@@ -599,33 +675,49 @@ bool celix_longHashMapIterator_isEnd(const
celix_long_hash_map_iterator_t* iter)
void celix_stringHashMapIterator_next(celix_string_hash_map_iterator_t* iter) {
const celix_hash_map_t* map = iter->_internal[0];
celix_hash_map_entry_t *entry = iter->_internal[1];
+iter->index += 1;
entry = celix_hashMap_nextEntry(map, entry);
-if (entry != NULL) {
+if (entry) {
iter->_internal[1] = entry;
-iter->index += 1;
iter->key = entry->key.strKey;
iter->value = entry->value;
} else {
-memset(iter, 0, sizeof(*iter));
-iter->_internal[0] = (void*)map;
+iter->_internal[1] = NULL;
+iter->key = NULL;
Review Comment:
nice catch, this should be aligned. Updated the next so that the key is set
to `""`.
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390439371
##
libs/utils/src/celix_hash_map.c:
##
@@ -237,11 +235,12 @@ static void celix_hashMap_addEntry(celix_hash_map_t* map,
unsigned int hash, con
newEntry->next = entry;
map->buckets[bucketIndex] = newEntry;
if (map->size++ >= celix_hashMap_threshold(map)) {
-celix_hashMap_resize(map, 2 * map->bucketsSize);
+return celix_hashMap_resize(map, 2 * map->bucketsSize);
Review Comment:
IMO it should fail, else we have a error which is not reported or logged to
celix_err without the users knowing a issue occurred.
Refactored the hashmap resize so that this is done before creating a new
entry. When resize fails - or when creating a new entry fails - users still
have ownership of weakly stored keys. I also updated the header dox for the
storeKeysWeakly to clarify this.
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390437669
##
libs/utils/src/celix_hash_map.c:
##
@@ -599,33 +675,49 @@ bool celix_longHashMapIterator_isEnd(const
celix_long_hash_map_iterator_t* iter)
void celix_stringHashMapIterator_next(celix_string_hash_map_iterator_t* iter) {
const celix_hash_map_t* map = iter->_internal[0];
celix_hash_map_entry_t *entry = iter->_internal[1];
+iter->index += 1;
Review Comment:
I designed the hashmap/properties iterator based on C++ iterator with
begin() and end(). For C++ the `end()` iterator is past the last element. So
IMO it is correct that the index of the end iterator is the same as the size of
the map.
##
libs/utils/src/celix_hash_map.c:
##
@@ -256,57 +255,46 @@ static bool celix_hashMap_putValue(celix_hash_map_t* map,
const char* strKey, lo
if (replacedValueOut != NULL) {
*replacedValueOut = entry->value;
}
+celix_hashMap_callRemovedCallback(map, entry);
memcpy(&entry->value, value, sizeof(*value));
-return true;
+return CELIX_SUCCESS;
}
}
-celix_hashMap_addEntry(map, hash, &key, value, index);
-if (replacedValueOut != NULL) {
+celix_status_t status = celix_hashMap_addEntry(map, hash, &key, value,
index);
+if (status == CELIX_SUCCESS && replacedValueOut != NULL) {
memset(replacedValueOut, 0, sizeof(*replacedValueOut));
}
-return false;
+return status;
}
-static void* celix_hashMap_put(celix_hash_map_t* map, const char* strKey, long
longKey, void* v) {
+static celix_status_t celix_hashMap_put(celix_hash_map_t* map, const char*
strKey, long longKey, void* v, void** previousValueOut) {
Review Comment:
removed
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390191593
##
libs/utils/src/version.c:
##
@@ -95,15 +98,18 @@ celix_status_t version_isCompatible(version_pt user,
version_pt provider, bool*
return CELIX_SUCCESS;
}
-celix_version_t* celix_version_createVersion(int major, int minor, int micro,
const char* qualifier) {
+celix_version_t* celix_version_create(int major, int minor, int micro, const
char* qualifier) {
if (major < 0 || minor < 0 || micro < 0) {
+celix_err_push("Invalid version number. Major, minor and micro must be
>= 0");
Review Comment:
Usage of ERR should be documented.
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390185418
##
libs/utils/include/celix_string_hash_map.h:
##
@@ -90,14 +94,38 @@ typedef struct celix_string_hash_map_create_options {
* only the simpledRemoveCallback will be used.
*
* Default is NULL.
+ *
+ * @param[in] data The void pointer to the data that was provided when the
callback was set as removedCallbackData.
+ * @param[in] removedKey The key of the value that was removed from the
hash map.
+ * Note that the removedKey can still be in use if
the a value entry for the same key is
+ * replaced, so this callback should not free the
removedKey.
+ * @param[in] removedValue The value that was removed from the hash map.
This value is no longer used by the
+ * hash map and can be freed.
*/
void (*removedCallback)(void* data, const char* removedKey,
celix_hash_map_value_t removedValue) CELIX_OPTS_INIT;
+/**
+ * @brief A removed key callback, which if provided will be called if a
key is no longer used in the hash map.
+ *
+ * @param[in] data The void pointer to the data that was provided when the
callback was set as removedCallbackData.
+ * @param[in] key The key that is no longer used in the hash map. if
`storeKeysWeakly` was configured as true,
+ *the key can be freed.
+ */
+void (*removedKeyCallback)(void* data, char* key) CELIX_OPTS_INIT;
+
/**
* @brief If set to true, the string hash map will not make of copy of the
keys and assumes
* that the keys are in scope/memory for the complete lifecycle of the
string hash map.
*
- * Note that this changes the default behaviour of the
celix_stringHashMap_put* functions.
+ * When keys are stored weakly it is the caller responsibility to check if
a key is already in use
+ * (celix_stringHashMap_hasKey).
+ *
+ * If the key is already in use, the celix_stringHashMap_put* will not
store the provided key and will
+ * instead keep the already existing key. If needed, the caller should
free the provided key.
Review Comment:
```suggestion
* instead keep the already existing key. If needed, the caller may free
the provided key.
```
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390184329
##
libs/utils/src/properties.c:
##
@@ -191,324 +481,449 @@ static void parseLine(const char* line,
celix_properties_t *props) {
}
if (!isComment) {
-//printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
+// printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
celix_properties_set(props, celix_utils_trimInPlace(key),
celix_utils_trimInPlace(value));
}
-if(key) {
+if (key) {
free(key);
-}
-if(value) {
free(value);
}
-
}
+celix_properties_t* celix_properties_loadWithStream(FILE* file) {
+if (file == NULL) {
+return NULL;
+}
-/**
-
**
- * Updated API
-
**
-
**/
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+if (!props) {
+celix_err_push("Failed to create properties");
+return NULL;
+}
+int rc = fseek(file, 0, SEEK_END);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to end of file. Got error %i", errno);
+return NULL;
+}
+size_t fileSize = ftell(file);
+rc = fseek(file, 0, SEEK_SET);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to start of file. Got error %i", errno);
+return NULL;
+}
+char* fileBuffer = malloc(fileSize + 1);
+if (fileBuffer == NULL) {
+celix_err_pushf("Cannot allocate memory for file buffer. Got error
%i", errno);
+return NULL;
+}
-celix_properties_t* celix_properties_create(void) {
-return hashMap_create(utils_stringHash, utils_stringHash,
utils_stringEquals, utils_stringEquals);
-}
+size_t rs = fread(fileBuffer, sizeof(char), fileSize, file);
+if (rs < fileSize) {
+fprintf(stderr, "fread read only %zu bytes out of %zu\n", rs,
fileSize);
+}
+fileBuffer[fileSize] = '\0'; // ensure a '\0' at the end of the fileBuffer
-void celix_properties_destroy(celix_properties_t *properties) {
-if (properties != NULL) {
-hash_map_iterator_pt iter = hashMapIterator_create(properties);
-while (hashMapIterator_hasNext(iter)) {
-hash_map_entry_pt entry = hashMapIterator_nextEntry(iter);
-hashMapEntry_clear(entry, true, true);
-}
-hashMapIterator_destroy(iter);
-hashMap_destroy(properties, false, false);
+char* savePtr = NULL;
+char* line = strtok_r(fileBuffer, "\n", &savePtr);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &savePtr);
}
+free(fileBuffer);
+
+return celix_steal_ptr(props);
}
-celix_properties_t* celix_properties_load(const char *filename) {
-FILE *file = fopen(filename, "r");
-if (file == NULL) {
+celix_properties_t* celix_properties_loadFromString(const char* input) {
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+celix_autofree char* in = celix_utils_strdup(input);
+if (!props || !in) {
+celix_err_push("Failed to create properties or duplicate input
string");
return NULL;
}
-celix_properties_t *props = celix_properties_loadWithStream(file);
-fclose(file);
-return props;
-}
-celix_properties_t* celix_properties_loadWithStream(FILE *file) {
-celix_properties_t *props = NULL;
-
-if (file != NULL ) {
-char *saveptr;
-char *filebuffer = NULL;
-char *line = NULL;
-ssize_t file_size = 0;
-
-props = celix_properties_create();
-fseek(file, 0, SEEK_END);
-file_size = ftell(file);
-fseek(file, 0, SEEK_SET);
+char* line = NULL;
+char* saveLinePointer = NULL;
+line = strtok_r(in, "\n", &saveLinePointer);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &saveLinePointer);
+}
+return celix_steal_ptr(props);
+}
-if (file_size > 0) {
-filebuffer = calloc(file_size + 1, sizeof(char));
-if (filebuffer) {
-size_t rs = fread(filebuffer, sizeof(char), file_size, file);
-if (rs != file_size) {
-fprintf(stderr,"fread read only %lu bytes out of %lu\n",
(long unsigned int) rs, (long unsigned int) file_size);
-}
-filebuffer[file_size]='\0'
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390182014
##
libs/utils/src/properties.c:
##
@@ -191,324 +481,449 @@ static void parseLine(const char* line,
celix_properties_t *props) {
}
if (!isComment) {
-//printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
+// printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
celix_properties_set(props, celix_utils_trimInPlace(key),
celix_utils_trimInPlace(value));
}
-if(key) {
+if (key) {
free(key);
-}
-if(value) {
free(value);
}
-
}
+celix_properties_t* celix_properties_loadWithStream(FILE* file) {
+if (file == NULL) {
+return NULL;
+}
-/**
-
**
- * Updated API
-
**
-
**/
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+if (!props) {
+celix_err_push("Failed to create properties");
+return NULL;
+}
+int rc = fseek(file, 0, SEEK_END);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to end of file. Got error %i", errno);
+return NULL;
+}
+size_t fileSize = ftell(file);
+rc = fseek(file, 0, SEEK_SET);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to start of file. Got error %i", errno);
+return NULL;
+}
+char* fileBuffer = malloc(fileSize + 1);
+if (fileBuffer == NULL) {
+celix_err_pushf("Cannot allocate memory for file buffer. Got error
%i", errno);
+return NULL;
+}
-celix_properties_t* celix_properties_create(void) {
-return hashMap_create(utils_stringHash, utils_stringHash,
utils_stringEquals, utils_stringEquals);
-}
+size_t rs = fread(fileBuffer, sizeof(char), fileSize, file);
+if (rs < fileSize) {
+fprintf(stderr, "fread read only %zu bytes out of %zu\n", rs,
fileSize);
+}
+fileBuffer[fileSize] = '\0'; // ensure a '\0' at the end of the fileBuffer
-void celix_properties_destroy(celix_properties_t *properties) {
-if (properties != NULL) {
-hash_map_iterator_pt iter = hashMapIterator_create(properties);
-while (hashMapIterator_hasNext(iter)) {
-hash_map_entry_pt entry = hashMapIterator_nextEntry(iter);
-hashMapEntry_clear(entry, true, true);
-}
-hashMapIterator_destroy(iter);
-hashMap_destroy(properties, false, false);
+char* savePtr = NULL;
+char* line = strtok_r(fileBuffer, "\n", &savePtr);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &savePtr);
}
+free(fileBuffer);
+
+return celix_steal_ptr(props);
}
-celix_properties_t* celix_properties_load(const char *filename) {
-FILE *file = fopen(filename, "r");
-if (file == NULL) {
+celix_properties_t* celix_properties_loadFromString(const char* input) {
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+celix_autofree char* in = celix_utils_strdup(input);
+if (!props || !in) {
+celix_err_push("Failed to create properties or duplicate input
string");
return NULL;
}
-celix_properties_t *props = celix_properties_loadWithStream(file);
-fclose(file);
-return props;
-}
-celix_properties_t* celix_properties_loadWithStream(FILE *file) {
-celix_properties_t *props = NULL;
-
-if (file != NULL ) {
-char *saveptr;
-char *filebuffer = NULL;
-char *line = NULL;
-ssize_t file_size = 0;
-
-props = celix_properties_create();
-fseek(file, 0, SEEK_END);
-file_size = ftell(file);
-fseek(file, 0, SEEK_SET);
+char* line = NULL;
+char* saveLinePointer = NULL;
+line = strtok_r(in, "\n", &saveLinePointer);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &saveLinePointer);
+}
+return celix_steal_ptr(props);
+}
-if (file_size > 0) {
-filebuffer = calloc(file_size + 1, sizeof(char));
-if (filebuffer) {
-size_t rs = fread(filebuffer, sizeof(char), file_size, file);
-if (rs != file_size) {
-fprintf(stderr,"fread read only %lu bytes out of %lu\n",
(long unsigned int) rs, (long unsigned int) file_size);
-}
-filebuffer[file_size]='\0'
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390163874
##
libs/utils/include/celix_properties.h:
##
@@ -17,86 +17,509 @@
* under the License.
*/
+/**
+ * @file celix_properties.h
+ * @brief Header file for the Celix Properties API.
+ *
+ * The Celix Properties API provides a means for storing and manipulating
key-value pairs, called properties,
+ * which can be used to store configuration data or metadata for a service,
component, or framework configuration.
+ * Functions are provided for creating and destroying property sets, loading
and storing properties from/to a file
+ * or stream, and setting, getting, and unsetting individual properties. There
are also functions for converting
+ * property values to various types (e.g. long, bool, double) and for
iterating over the properties in a set.
+ *
+ * Supported property value types include:
+ * - string (char*)
+ * - long
+ * - double
+ * - bool
+ * - celix_version_t*
+ */
+
#ifndef CELIX_PROPERTIES_H_
#define CELIX_PROPERTIES_H_
#include
-#include
#include "celix_cleanup.h"
#include "celix_compiler.h"
#include "celix_errno.h"
#include "celix_utils_export.h"
+#include "celix_version.h"
#ifdef __cplusplus
extern "C" {
#endif
-typedef struct hashMap celix_properties_t; //opaque struct, TODO try to make
this a celix_properties struct
+/**
+ * @brief celix_properties_t is a type that represents a set of key-value
pairs called properties.
+ *
+ * @note Not thread safe.
+ */
+typedef struct celix_properties celix_properties_t;
+
+/**
+ * @brief Enum representing the possible types of a property value.
+ */
+typedef enum celix_properties_value_type {
+CELIX_PROPERTIES_VALUE_TYPE_UNSET = 0, /**< Property value is not set. */
+CELIX_PROPERTIES_VALUE_TYPE_STRING = 1, /**< Property value is a string. */
+CELIX_PROPERTIES_VALUE_TYPE_LONG = 2, /**< Property value is a long
integer. */
+CELIX_PROPERTIES_VALUE_TYPE_DOUBLE = 3, /**< Property value is a double. */
+CELIX_PROPERTIES_VALUE_TYPE_BOOL = 4, /**< Property value is a boolean.
*/
+CELIX_PROPERTIES_VALUE_TYPE_VERSION = 5 /**< Property value is a Celix
version. */
+} celix_properties_value_type_e;
+/**
+ * @brief A structure representing a single value entry in a property set.
+ */
+typedef struct celix_properties_entry {
+const char* value; /**< The string value or string
representation of a non-string
+ typed value.*/
+celix_properties_value_type_e valueType; /**< The type of the value of the
entry */
+
+union {
+const char* strValue;/**< The string value of the
entry. */
+long longValue; /**< The long integer value of
the entry. */
+double doubleValue; /**< The double-precision
floating point value of the entry. */
+bool boolValue; /**< The boolean value of the
entry. */
+const celix_version_t* versionValue; /**< The Celix version value of
the entry. */
+} typed; /**< The typed values of the
entry. Only valid if valueType
+ is not
CELIX_PROPERTIES_VALUE_TYPE_UNSET and only the matching
+ value types should be used.
E.g typed.boolValue if valueType is
+
CELIX_PROPERTIES_VALUE_TYPE_BOOL. */
+} celix_properties_entry_t;
+
+/**
+ * @brief Represents an iterator for iterating over the entries in a
celix_properties_t object.
+ */
typedef struct celix_properties_iterator {
-//private data
-void* _data1;
-void* _data2;
-void* _data3;
-int _data4;
-int _data5;
-} celix_properties_iterator_t;
+/**
+ * @brief The index of the current iterator.
+ */
+size_t index;
+/**
+ * @brief The current key.
+ */
+const char* key;
-/**
-
**
- * Updated API
-
**
-
**/
+/**
+ * @brief The current value entry.
+ */
+celix_properties_entry_t entry;
-CELIX_UTILS_EXPORT celix_properties_t* celix_properties_create(void);
+/**
+ * @brief Private data used to implement the iterator.
+ */
+char _data[56];
+} celix_properties_iterator_t;
-CELIX_UTILS_EXPORT void celix_properties_destroy(celix_properties_t
*properties);
+/**
+ * @brief Create a new empty property set
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390163874
##
libs/utils/include/celix_properties.h:
##
@@ -17,86 +17,509 @@
* under the License.
*/
+/**
+ * @file celix_properties.h
+ * @brief Header file for the Celix Properties API.
+ *
+ * The Celix Properties API provides a means for storing and manipulating
key-value pairs, called properties,
+ * which can be used to store configuration data or metadata for a service,
component, or framework configuration.
+ * Functions are provided for creating and destroying property sets, loading
and storing properties from/to a file
+ * or stream, and setting, getting, and unsetting individual properties. There
are also functions for converting
+ * property values to various types (e.g. long, bool, double) and for
iterating over the properties in a set.
+ *
+ * Supported property value types include:
+ * - string (char*)
+ * - long
+ * - double
+ * - bool
+ * - celix_version_t*
+ */
+
#ifndef CELIX_PROPERTIES_H_
#define CELIX_PROPERTIES_H_
#include
-#include
#include "celix_cleanup.h"
#include "celix_compiler.h"
#include "celix_errno.h"
#include "celix_utils_export.h"
+#include "celix_version.h"
#ifdef __cplusplus
extern "C" {
#endif
-typedef struct hashMap celix_properties_t; //opaque struct, TODO try to make
this a celix_properties struct
+/**
+ * @brief celix_properties_t is a type that represents a set of key-value
pairs called properties.
+ *
+ * @note Not thread safe.
+ */
+typedef struct celix_properties celix_properties_t;
+
+/**
+ * @brief Enum representing the possible types of a property value.
+ */
+typedef enum celix_properties_value_type {
+CELIX_PROPERTIES_VALUE_TYPE_UNSET = 0, /**< Property value is not set. */
+CELIX_PROPERTIES_VALUE_TYPE_STRING = 1, /**< Property value is a string. */
+CELIX_PROPERTIES_VALUE_TYPE_LONG = 2, /**< Property value is a long
integer. */
+CELIX_PROPERTIES_VALUE_TYPE_DOUBLE = 3, /**< Property value is a double. */
+CELIX_PROPERTIES_VALUE_TYPE_BOOL = 4, /**< Property value is a boolean.
*/
+CELIX_PROPERTIES_VALUE_TYPE_VERSION = 5 /**< Property value is a Celix
version. */
+} celix_properties_value_type_e;
+/**
+ * @brief A structure representing a single value entry in a property set.
+ */
+typedef struct celix_properties_entry {
+const char* value; /**< The string value or string
representation of a non-string
+ typed value.*/
+celix_properties_value_type_e valueType; /**< The type of the value of the
entry */
+
+union {
+const char* strValue;/**< The string value of the
entry. */
+long longValue; /**< The long integer value of
the entry. */
+double doubleValue; /**< The double-precision
floating point value of the entry. */
+bool boolValue; /**< The boolean value of the
entry. */
+const celix_version_t* versionValue; /**< The Celix version value of
the entry. */
+} typed; /**< The typed values of the
entry. Only valid if valueType
+ is not
CELIX_PROPERTIES_VALUE_TYPE_UNSET and only the matching
+ value types should be used.
E.g typed.boolValue if valueType is
+
CELIX_PROPERTIES_VALUE_TYPE_BOOL. */
+} celix_properties_entry_t;
+
+/**
+ * @brief Represents an iterator for iterating over the entries in a
celix_properties_t object.
+ */
typedef struct celix_properties_iterator {
-//private data
-void* _data1;
-void* _data2;
-void* _data3;
-int _data4;
-int _data5;
-} celix_properties_iterator_t;
+/**
+ * @brief The index of the current iterator.
+ */
+size_t index;
+/**
+ * @brief The current key.
+ */
+const char* key;
-/**
-
**
- * Updated API
-
**
-
**/
+/**
+ * @brief The current value entry.
+ */
+celix_properties_entry_t entry;
-CELIX_UTILS_EXPORT celix_properties_t* celix_properties_create(void);
+/**
+ * @brief Private data used to implement the iterator.
+ */
+char _data[56];
+} celix_properties_iterator_t;
-CELIX_UTILS_EXPORT void celix_properties_destroy(celix_properties_t
*properties);
+/**
+ * @brief Create a new empty property set
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1389464529
##
libs/utils/include/celix_properties.h:
##
@@ -17,86 +17,474 @@
* under the License.
*/
-#ifndef CELIX_PROPERTIES_H_
-#define CELIX_PROPERTIES_H_
+/**
+ * @file celix_properties.h
+ * @brief Header file for the Celix Properties API.
+ *
+ * The Celix Properties API provides a means for storing and manipulating
key-value pairs, called properties,
+ * which can be used to store configuration data or metadata for a service,
component, or framework configuration.
+ * Functions are provided for creating and destroying property sets, loading
and storing properties from/to a file
+ * or stream, and setting, getting, and unsetting individual properties. There
are also functions for converting
+ * property values to various types (e.g. long, bool, double) and for
iterating over the properties in a set.
+ *
+ * Supported property value types include:
+ * - string (char*)
+ * - long
+ * - double
+ * - bool
+ * - celix_version_t*
+ */
#include
-#include
#include "celix_cleanup.h"
#include "celix_compiler.h"
#include "celix_errno.h"
#include "celix_utils_export.h"
+#include "celix_version.h"
+
+#ifndef CELIX_PROPERTIES_H_
+#define CELIX_PROPERTIES_H_
#ifdef __cplusplus
extern "C" {
#endif
-typedef struct hashMap celix_properties_t; //opaque struct, TODO try to make
this a celix_properties struct
+/**
+ * @brief celix_properties_t is a type that represents a set of key-value
pairs called properties.
+ *
+ * @note Not thread safe.
+ */
+typedef struct celix_properties celix_properties_t;
+
+/**
+ * @brief Enum representing the possible types of a property value.
+ */
+typedef enum celix_properties_value_type {
+CELIX_PROPERTIES_VALUE_TYPE_UNSET = 0, /**< Property value is not set. */
+CELIX_PROPERTIES_VALUE_TYPE_STRING = 1, /**< Property value is a string. */
+CELIX_PROPERTIES_VALUE_TYPE_LONG = 2, /**< Property value is a long
integer. */
+CELIX_PROPERTIES_VALUE_TYPE_DOUBLE = 3, /**< Property value is a double. */
+CELIX_PROPERTIES_VALUE_TYPE_BOOL = 4, /**< Property value is a boolean.
*/
+CELIX_PROPERTIES_VALUE_TYPE_VERSION = 5 /**< Property value is a Celix
version. */
+} celix_properties_value_type_e;
+
+/**
+ * @brief A structure representing a single value entry in a property set.
+ */
+typedef struct celix_properties_entry {
+const char* value; /**< The string value or string
representation of a non-string
+ typed value.*/
+celix_properties_value_type_e valueType; /**< The type of the value of the
entry */
+union {
+const char* strValue;/**< The string value of the
entry. */
+long longValue; /**< The long integer value of
the entry. */
+double doubleValue; /**< The double-precision
floating point value of the entry. */
+bool boolValue; /**< The boolean value of the
entry. */
+const celix_version_t* versionValue; /**< The Celix version value of
the entry. */
+} typed; /**< The typed values of the
entry. Only valid if valueType
+ is not
CELIX_PROPERTIES_VALUE_TYPE_UNSET and only the matching
+ value types should be used.
E.g typed.boolValue if valueType is
+
CELIX_PROPERTIES_VALUE_TYPE_BOOL. */
+} celix_properties_entry_t;
+
+/**
+ * @brief Represents an iterator for iterating over the entries in a
celix_properties_t object.
+ */
typedef struct celix_properties_iterator {
-//private data
-void* _data1;
-void* _data2;
-void* _data3;
-int _data4;
-int _data5;
-} celix_properties_iterator_t;
+/**
+ * @brief The index of the current iterator.
+ */
+size_t index;
+/**
+ * @brief Te current key.
+ */
+const char* key;
-/**
-
**
- * Updated API
-
**
-
**/
+/**
+ * @brief The current value entry.
+ */
+celix_properties_entry_t entry;
-CELIX_UTILS_EXPORT celix_properties_t* celix_properties_create(void);
+/**
+ * @brief Private data used to implement the iterator.
+ */
+char _data[56];
+} celix_properties_iterator_t;
-CELIX_UTILS_EXPORT void celix_properties_destroy(celix_properties_t
*prope
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1389455788
##
libs/utils/include/celix_string_hash_map.h:
##
@@ -90,14 +94,37 @@ typedef struct celix_string_hash_map_create_options {
* only the simpledRemoveCallback will be used.
*
* Default is NULL.
+ *
+ * @param[in] data The void pointer to the data that was provided when the
callback was set as removedCallbackData.
+ * @param[in] removedKey The key of the value that was removed from the
hash map.
+ * Note that the removedKey can still be in use if
the a value entry for the same key is
+ * replaced, so this callback should not free the
removedKey.
+ * @param[in] removedValue The value that was removed from the hash map.
This value is no longer used by the
+ * hash map and can be freed.
*/
void (*removedCallback)(void* data, const char* removedKey,
celix_hash_map_value_t removedValue) CELIX_OPTS_INIT;
+/**
+ * @brief A removed key callback, which if provided will be called if a
key is no longer used in the hash map.
+ *
+ * @param[in] data The void pointer to the data that was provided when the
callback was set as removedCallbackData.
+ * @param[in] key The key that is no longer used in the hash map. if
`storeKeysWeakly` was configured as true,
+ *the key can be freed.
+ */
+void (*removedKeyCallback)(void* data, char* key) CELIX_OPTS_INIT;
+
/**
* @brief If set to true, the string hash map will not make of copy of the
keys and assumes
* that the keys are in scope/memory for the complete lifecycle of the
string hash map.
*
- * Note that this changes the default behaviour of the
celix_stringHashMap_put* functions.
+ * When keys are stored weakly it is the caller responsibility to check
the return value of
+ * celix_stringHashMap_put* function calls.
+ * If a celix_stringHashMap_put* function call returns true, the key is
used in the hash map and the key
+ * should never be freed or freed in a configured removedKeyCallback.
+ * If a celix_stringHashMap_put* function call returns false, a value is
replaced and the already existing
+ * key is reused. If the needed the caller should free the provided key.
Review Comment:
I updated the documentation to clarify that the users should first check if
a key exists with `celix_stringHashMap_hasKey`. This will lead to some more
code, but gives users the flexibility - if possible - to only allocate a key if
needed (as is done in properties).
It is also possible to call the removeKeyCallback for every not used key,
but I think this does not really work with the short properties string
optimization buffer.
An other option could be to add a set of `celix_stringHashMap_replace*`
functions, with a boolean output argument.
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1389419559
##
libs/utils/include/celix/Version.h:
##
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include
+#include
+
+#include "celix_version.h"
+
+namespace celix {
+
+/**
+ * @class Version
+ * @brief Class for storing and manipulating version information.
+ *
+ * The Version class represents a version number that follows the Semantic
Versioning specification (SemVer).
+ * It consists of three non-negative integers for the major, minor, and
micro version, and an optional string for
+ * the qualifier.
+ * The Version class provides comparison operators and functions for
getting the individual version components.
+ *
+ * @note This class is a thin wrapper around the C API defined in
celix_version.h.
+ */
+class Version {
+public:
+
+///@brief Constructs a new empty version with all components set to
zero.
+Version() :
+cVersion{createVersion(celix_version_createEmptyVersion())},
+qualifier{celix_version_getQualifier(cVersion.get())} {}
+
+/**
+ * @brief Constructs a new version with the given components and
qualifier.
+ * @param major The major component of the version.
+ * @param minor The minor component of the version.
+ * @param micro The micro component of the version.
+ * @param qualifier The qualifier string of the version.
+ */
+#if __cplusplus >= 201703L //C++17 or higher
+Version(int major, int minor, int micro, std::string_view qualifier =
{}) :
+cVersion{createVersion(celix_version_create(major, minor, micro,
qualifier.empty() ? "" : qualifier.data()))},
+qualifier{celix_version_getQualifier(cVersion.get())} {}
+#else
+explicit Version(int major, int minor, int micro, const std::string&
qualifier = {}) :
+cVersion{createVersion(celix_version_create(major, minor, micro,
qualifier.empty() ? "" : qualifier.c_str()))},
+qualifier{celix_version_getQualifier(cVersion.get())} {}
+#endif
+
+///@brief Move-constructs a new version from an existing one.
+Version(Version&&) = default;
Review Comment:
same as with Properties. cVersion will be nullptr and as far as I know that
is ok for a move.
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1389375314
##
libs/utils/src/properties.c:
##
@@ -103,12 +132,275 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const char** strValue,
+ const long* longValue,
+ const double* doubleValue,
+ const bool* boolValue,
+ celix_version_t*
versionValue) {
+char convertedValueBuffer[32];
+if (strValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING;
+entry->value = celix_properties_createString(properties, *strValue);
+entry->typed.strValue = entry->value;
+} else if (longValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG;
+entry->typed.longValue = *longValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%li", entry->typed.longValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%li", entry->typed.longValue);
+entry->value = val;
+}
+} else if (doubleValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE;
+entry->typed.doubleValue = *doubleValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%f", entry->typed.doubleValue);
+entry->value = val;
+}
+} else if (boolValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL;
+entry->typed.boolValue = *boolValue;
+entry->value = entry->typed.boolValue ?
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+} else /*versionValue*/ {
+assert(versionValue != NULL);
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
+entry->typed.versionValue = versionValue;
+bool written = celix_version_fillString(versionValue,
convertedValueBuffer, sizeof(convertedValueBuffer));
+if (written) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+entry->value = celix_version_toString(versionValue);
+}
+}
+if (entry->value == NULL) {
+return CELIX_ENOMEM;
+}
+return CELIX_SUCCESS;
+}
+
+/**
+ * Allocate entry and optionally use the short properties optimization entries
buffer.
+ */
+celix_propertie
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1389333404
##
libs/utils/include/celix/Properties.h:
##
@@ -145,20 +172,19 @@ namespace celix {
};
-Properties() : cProps{celix_properties_create(),
[](celix_properties_t* p) { celix_properties_destroy(p); }} {}
+Properties() : cProps{createCProps(celix_properties_create())} {}
Properties(Properties&&) = default;
Review Comment:
Yes, I think cProps will then be nullptr.. but as for as I know a variable
to a moved object should not be used, expect to potentially reassign a new
object. or reinitialized the object.
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1389236742
##
libs/utils/gtest/src/HashMapTestSuite.cc:
##
@@ -534,25 +514,159 @@ TEST_F(HashMapTestSuite, IterateWithRemoveTest) {
auto iter2 = celix_longHashMap_begin(lMap);
while (!celix_longHashMapIterator_isEnd(&iter2)) {
if (iter2.index % 2 == 0) {
-//note only removing entries where the iter index is even
+// note only removing entries where the iter index is even
celix_longHashMapIterator_remove(&iter2);
} else {
celix_longHashMapIterator_next(&iter2);
}
}
EXPECT_EQ(3, celix_longHashMap_size(lMap));
EXPECT_TRUE(celix_longHashMapIterator_isEnd(&iter2));
-celix_longHashMapIterator_next(&iter2); //calling next on end iter, does
nothing
+celix_longHashMapIterator_next(&iter2); // calling next on end iter, does
nothing
EXPECT_TRUE(celix_longHashMapIterator_isEnd(&iter2));
celix_longHashMap_destroy(lMap);
}
-TEST_F(HashMapTestSuite, StringHashMapCleanup) {
+TEST_F(HashMapTestSuite, IterateEndTest) {
+auto* sMap1 = createStringHashMap(0);
+auto* sMap2 = createStringHashMap(6);
+auto* lMap1 = createLongHashMap(0);
+auto* lMap2 = createLongHashMap(6);
+
+auto sIter1 = celix_stringHashMap_end(sMap1);
+auto sIter2 = celix_stringHashMap_end(sMap2);
+auto lIter1 = celix_longHashMap_end(lMap1);
+auto lIter2 = celix_longHashMap_end(lMap2);
+
+EXPECT_EQ(sIter1.index, 0);
+EXPECT_EQ(sIter2.index, 6);
+EXPECT_EQ(lIter1.index, 0);
+EXPECT_EQ(lIter2.index, 6);
+EXPECT_TRUE(celix_stringHashMapIterator_isEnd(&sIter1));
+EXPECT_TRUE(celix_stringHashMapIterator_isEnd(&sIter2));
+EXPECT_TRUE(celix_longHashMapIterator_isEnd(&lIter1));
+EXPECT_TRUE(celix_longHashMapIterator_isEnd(&lIter2));
+
+celix_stringHashMap_destroy(sMap1);
+celix_stringHashMap_destroy(sMap2);
+celix_longHashMap_destroy(lMap1);
+celix_longHashMap_destroy(lMap2);
+}
+
+TEST_F(HashMapTestSuite, EqualsTest) {
+auto* sMap = createStringHashMap(2);
+auto sIter1 = celix_stringHashMap_begin(sMap);
+auto sIter2 = celix_stringHashMap_begin(sMap);
+
+// Test equal iterators
+EXPECT_TRUE(celix_stringHashMapIterator_equals(&sIter1, &sIter2));
+
+// Test unequal iterators after only 1 modification
+celix_stringHashMapIterator_next(&sIter1);
+EXPECT_FALSE(celix_stringHashMapIterator_equals(&sIter1, &sIter2));
+
+// Test equal iterators after both modification
+celix_stringHashMapIterator_next(&sIter2);
+EXPECT_TRUE(celix_stringHashMapIterator_equals(&sIter1, &sIter2));
+
+// Same for long hash map
+auto* lMap = createLongHashMap(1);
+auto lIter1 = celix_longHashMap_begin(lMap);
+auto lIter2 = celix_longHashMap_begin(lMap);
+
+// Test equal iterators
+EXPECT_TRUE(celix_longHashMapIterator_equals(&lIter1, &lIter2));
+
+// Test unequal iterators after only 1 modification
+celix_longHashMapIterator_next(&lIter1);
+EXPECT_FALSE(celix_longHashMapIterator_equals(&lIter1, &lIter2));
+
+// Test equal iterators after both modification
+celix_longHashMapIterator_next(&lIter2);
+EXPECT_TRUE(celix_longHashMapIterator_equals(&lIter1, &lIter2));
+
+celix_stringHashMap_destroy(sMap);
+celix_longHashMap_destroy(lMap);
+}
+
+TEST_F(HashMapTestSuite, EqualsZeroSizeMapTest) {
+// Because map size is 0, begin iter should equal end iter
+auto* sMap = createStringHashMap(0);
+auto sIter1 = celix_stringHashMap_begin(sMap);
+auto sEnd = celix_stringHashMap_end(sMap);
+EXPECT_TRUE(celix_stringHashMapIterator_equals(&sIter1, &sEnd));
+
+// Because map size is 0, begin iter should equal end iter
+auto* lMap = createLongHashMap(0);
+auto lIter1 = celix_longHashMap_begin(lMap);
+auto lEnd = celix_longHashMap_end(lMap);
+EXPECT_TRUE(celix_longHashMapIterator_equals(&lIter1, &lEnd));
+
+celix_stringHashMap_destroy(sMap);
+celix_longHashMap_destroy(lMap);
+}
+
+TEST_F(HashMapTestSuite, StoreKeysWeaklyTest) {
+celix_string_hash_map_create_options_t opts{};
+opts.removedCallbackData = (void*)0x1;
+opts.storeKeysWeakly = true;
+opts.removedKeyCallback = [](void* data, char* key) {
+EXPECT_EQ(data, (void*)0x1);
+free(key);
+};
+auto* sMap = celix_stringHashMap_createWithOptions(&opts);
+EXPECT_FALSE(celix_stringHashMap_putLong(sMap, celix_utils_strdup("key1"),
1)); // new key -> takes ownership
Review Comment:
The tests have been updated for the changed return type.
But I think the `pts.removedKeyCallback` documentation is still valid and
not changed, or am I missing something?
--
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.
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1389228884
##
libs/utils/gtest/src/CxxPropertiesTestSuite.cc:
##
@@ -52,14 +52,37 @@ TEST_F(CxxPropertiesTestSuite, testFillAndLoop) {
EXPECT_EQ(props.getAsBool("key5", false), true);
int count = 0;
+for (auto it = props.begin(); it != props.end(); ++it) {
Review Comment:
The following range based test used a const ref, so uses `cbegin()` and
`cend()`. This way we test 2 styles and both the const and mutable variant.
--
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]
Re: [PR] type support for properties (celix)
xuzhenbao commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1378321550
##
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:
I think `celix_propertiesIterator_next` also should be invoked before `while
(!celix_propertiesIterator_isEnd(&propIter))` . otherwise, a same property is
set in `txtRecord`.
We can resolve it in another way: Only let
`discoveryZeroconfAnnouncer_copyPropertiesToTxtRecord` do
`celix_propertiesIterator_next` whether txtRecord is filled or not.
~~~
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;
while (!celix_propertiesIterator_isEnd(propIter)) {
key = propIter->key;
val = propIter->entry.value;
if (key) {
DNSServiceErrorType err = TXTRecordSetValue(txtRecord, key,
strlen(val), val);
if (err != kDNSServiceErr_NoError) {
celix_logHelper_error(announcer->logHelper, "Announcer:
Failed to set txt value, %d.", err);
return false;
}
if (splitTxtRecord && TXTRecordGetLength(txtRecord) >= maxTxtLen
- UINT8_MAX) {
celix_propertiesIterator_next(propIter);
break;
}
}
celix_propertiesIterator_next(propIter);
}
return true;
}
~~~
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1388473469
##
libs/utils/src/properties.c:
##
@@ -103,12 +132,275 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strdup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const char** strValue,
+ const long* longValue,
+ const double* doubleValue,
+ const bool* boolValue,
+ celix_version_t*
versionValue) {
+char convertedValueBuffer[32];
+if (strValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING;
+entry->value = celix_properties_createString(properties, *strValue);
+entry->typed.strValue = entry->value;
+} else if (longValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG;
+entry->typed.longValue = *longValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%li", entry->typed.longValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%li", entry->typed.longValue);
+entry->value = val;
+}
+} else if (doubleValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE;
+entry->typed.doubleValue = *doubleValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%f", entry->typed.doubleValue);
+entry->value = val;
+}
+} else if (boolValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL;
+entry->typed.boolValue = *boolValue;
+entry->value = entry->typed.boolValue ?
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+} else /*versionValue*/ {
+assert(versionValue != NULL);
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
+entry->typed.versionValue = versionValue;
+bool written = celix_version_fillString(versionValue,
convertedValueBuffer, sizeof(convertedValueBuffer));
+if (written) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+entry->value = celix_version_toString(versionValue);
+}
+}
+if (entry->value == NULL) {
+return CELIX_ENOMEM;
+}
+return CELIX_SUCCESS;
+}
+
+/**
+ * Allocate entry and optionally use the short properties optimization entries
buffer.
+ */
+celix_properti
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470: URL: https://github.com/apache/celix/pull/470#discussion_r1388340942 ## libs/utils/src/properties.c: ## @@ -17,55 +17,87 @@ * under the License. */ +#include "properties.h" +#include "celix_properties.h" +#include "celix_properties_private.h" + +#include +#include +#include +#include #include -#include #include -#include -#include +#include -#include "properties.h" #include "celix_build_assert.h" -#include "celix_properties.h" +#include "celix_err.h" +#include "celix_string_hash_map.h" #include "celix_utils.h" -#include "utils.h" -#include "hash_map_private.h" -#include -#include "hash_map.h" +#include "celix_stdlib_cleanup.h" +#include "celix_convert_utils.h" -#define MALLOC_BLOCK_SIZE5 +#define CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE 512 Review Comment: I will make this configurable (as is done with `celix_err_buffer_size`) and set the default to 128. I also created a issue to #684 so that users can get some statistics about the service framework and use that to determine (if needed) the ideal properties optimization size for their use cases. -- 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]
Re: [PR] type support for properties (celix)
pnoltes commented on PR #470: URL: https://github.com/apache/celix/pull/470#issuecomment-1804253059 > Here are some remarks on properties implementation, and I have finished the first round review, which takes much longer than expected. That is understandable: for such a change of fundamental type used everywhere, we need to be extra careful to avoid introducing bugs. I tried my best. > > Overall, it's a nice work which brings about huge performance and ergonomics improvements. Looking forward to getting it merged and implementing #674 thereafter. Thanks for the thorough review :smile: . I will update the implementation based on the review comments. -- 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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1384609156
##
libs/utils/src/properties.c:
##
@@ -191,324 +476,423 @@ static void parseLine(const char* line,
celix_properties_t *props) {
}
if (!isComment) {
-//printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
+// printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
celix_properties_set(props, celix_utils_trimInPlace(key),
celix_utils_trimInPlace(value));
}
-if(key) {
+if (key) {
free(key);
-}
-if(value) {
free(value);
}
-
}
+celix_properties_t* celix_properties_loadWithStream(FILE* file) {
+if (file == NULL) {
+return NULL;
+}
-/**
-
**
- * Updated API
-
**
-
**/
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+if (!props) {
+celix_err_push("Failed to create properties");
+return NULL;
+}
+int rc = fseek(file, 0, SEEK_END);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to end of file. Got error %i", errno);
+return NULL;
+}
+size_t fileSize = ftell(file);
+rc = fseek(file, 0, SEEK_SET);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to start of file. Got error %i", errno);
+return NULL;
+}
+char* fileBuffer = malloc(fileSize + 1);
+if (fileBuffer == NULL) {
+celix_err_pushf("Cannot allocate memory for file buffer. Got error
%i", errno);
+return NULL;
+}
-celix_properties_t* celix_properties_create(void) {
-return hashMap_create(utils_stringHash, utils_stringHash,
utils_stringEquals, utils_stringEquals);
-}
+size_t rs = fread(fileBuffer, sizeof(char), fileSize, file);
+if (rs < fileSize) {
+fprintf(stderr, "fread read only %zu bytes out of %zu\n", rs,
fileSize);
+}
+fileBuffer[fileSize] = '\0'; // ensure a '\0' at the end of the fileBuffer
-void celix_properties_destroy(celix_properties_t *properties) {
-if (properties != NULL) {
-hash_map_iterator_pt iter = hashMapIterator_create(properties);
-while (hashMapIterator_hasNext(iter)) {
-hash_map_entry_pt entry = hashMapIterator_nextEntry(iter);
-hashMapEntry_clear(entry, true, true);
-}
-hashMapIterator_destroy(iter);
-hashMap_destroy(properties, false, false);
+char* savePtr = NULL;
+char* line = strtok_r(fileBuffer, "\n", &savePtr);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &savePtr);
}
+free(fileBuffer);
+
+return celix_steal_ptr(props);
}
-celix_properties_t* celix_properties_load(const char *filename) {
-FILE *file = fopen(filename, "r");
-if (file == NULL) {
+celix_properties_t* celix_properties_loadFromString(const char* input) {
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+celix_autofree char* in = celix_utils_strdup(input);
+if (!props || !in) {
+celix_err_push("Failed to create properties or duplicate input
string");
return NULL;
}
-celix_properties_t *props = celix_properties_loadWithStream(file);
-fclose(file);
-return props;
-}
-
-celix_properties_t* celix_properties_loadWithStream(FILE *file) {
-celix_properties_t *props = NULL;
-if (file != NULL ) {
-char *saveptr;
-char *filebuffer = NULL;
-char *line = NULL;
-ssize_t file_size = 0;
-
-props = celix_properties_create();
-fseek(file, 0, SEEK_END);
-file_size = ftell(file);
-fseek(file, 0, SEEK_SET);
+char* line = NULL;
+char* saveLinePointer = NULL;
+line = strtok_r(in, "\n", &saveLinePointer);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &saveLinePointer);
+}
+return celix_steal_ptr(props);
+}
-if (file_size > 0) {
-filebuffer = calloc(file_size + 1, sizeof(char));
-if (filebuffer) {
-size_t rs = fread(filebuffer, sizeof(char), file_size, file);
-if (rs != file_size) {
-fprintf(stderr,"fread read only %lu bytes out of %lu\n",
(long unsigned int) rs, (long unsigned int) file_size);
-}
-filebuffer[file_size]='\0'
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470: URL: https://github.com/apache/celix/pull/470#discussion_r1383337330 ## libs/utils/src/properties.c: ## @@ -17,55 +17,87 @@ * under the License. */ +#include "properties.h" +#include "celix_properties.h" +#include "celix_properties_private.h" + +#include +#include +#include +#include #include -#include #include -#include -#include +#include -#include "properties.h" #include "celix_build_assert.h" -#include "celix_properties.h" +#include "celix_err.h" +#include "celix_string_hash_map.h" #include "celix_utils.h" -#include "utils.h" -#include "hash_map_private.h" -#include -#include "hash_map.h" +#include "celix_stdlib_cleanup.h" +#include "celix_convert_utils.h" -#define MALLOC_BLOCK_SIZE5 +#define CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE 512 Review Comment: Short properties optimization is a good idea. I am not sure whether the size should be set according to max values. If this is enough for 80% usages, then I consider it a good choice. Given that celix_properties are used extensively and for various purposes other than service metadata, 128/256 may seem less scary. -- 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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1383324230
##
libs/utils/src/properties.c:
##
@@ -17,55 +17,87 @@
* under the License.
*/
+#include "properties.h"
+#include "celix_properties.h"
+#include "celix_properties_private.h"
+
+#include
+#include
+#include
+#include
#include
-#include
#include
-#include
-#include
+#include
-#include "properties.h"
#include "celix_build_assert.h"
-#include "celix_properties.h"
+#include "celix_err.h"
+#include "celix_string_hash_map.h"
#include "celix_utils.h"
-#include "utils.h"
-#include "hash_map_private.h"
-#include
-#include "hash_map.h"
+#include "celix_stdlib_cleanup.h"
+#include "celix_convert_utils.h"
-#define MALLOC_BLOCK_SIZE5
+#define CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE 512
+#define CELIX_SHORT_PROPERTIES_OPTIMIZATION_ENTRIES_SIZE 16
-static void parseLine(const char* line, celix_properties_t *props);
+static const char* const CELIX_PROPERTIES_BOOL_TRUE_STRVAL = "true";
+static const char* const CELIX_PROPERTIES_BOOL_FALSE_STRVAL = "false";
+static const char* const CELIX_PROPERTIES_EMPTY_STRVAL = "";
-properties_pt properties_create(void) {
-return celix_properties_create();
-}
+struct celix_properties {
+celix_string_hash_map_t* map;
-void properties_destroy(properties_pt properties) {
-celix_properties_destroy(properties);
-}
+/**
+ * String buffer used to store the first key/value entries,
+ * so that in many cases - for usage in service properties - additional
memory allocations are not needed.
+ *
+ * @note based on some small testing most services properties seem to max
out at 11 entries.
Review Comment:
This note should apply to `entryBuffer`?
##
libs/utils/src/properties.c:
##
@@ -103,12 +132,275 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const char** strValue,
+ const long* longValue,
+ const double* doubleValue,
+ const bool* boolValue,
+ celix_version_t*
versionValue) {
+char convertedValueBuffer[32];
+if (strValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING;
+entry->value = celix_properties_createString(properties, *strValue);
+entry->typed.strValue = entry->value;
+} else if (longValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG;
+entry->typed.longValue = *longValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%li", entry->typed.longValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1383248138
##
libs/utils/src/celix_hash_map.c:
##
@@ -237,11 +235,12 @@ static void celix_hashMap_addEntry(celix_hash_map_t* map,
unsigned int hash, con
newEntry->next = entry;
map->buckets[bucketIndex] = newEntry;
if (map->size++ >= celix_hashMap_threshold(map)) {
-celix_hashMap_resize(map, 2 * map->bucketsSize);
+return celix_hashMap_resize(map, 2 * map->bucketsSize);
Review Comment:
if `newEntry` is added but `celix_hashMap_resize` failed, should it return
SUCCESS or error code?
##
libs/utils/src/celix_hash_map.c:
##
@@ -599,33 +675,49 @@ bool celix_longHashMapIterator_isEnd(const
celix_long_hash_map_iterator_t* iter)
void celix_stringHashMapIterator_next(celix_string_hash_map_iterator_t* iter) {
const celix_hash_map_t* map = iter->_internal[0];
celix_hash_map_entry_t *entry = iter->_internal[1];
+iter->index += 1;
entry = celix_hashMap_nextEntry(map, entry);
-if (entry != NULL) {
+if (entry) {
iter->_internal[1] = entry;
-iter->index += 1;
iter->key = entry->key.strKey;
iter->value = entry->value;
} else {
-memset(iter, 0, sizeof(*iter));
-iter->_internal[0] = (void*)map;
+iter->_internal[1] = NULL;
+iter->key = NULL;
Review Comment:
In `celix_stringHashMap_end`, `key` is set to "".
##
libs/utils/src/celix_hash_map.c:
##
@@ -339,21 +327,27 @@ static bool celix_hashMap_remove(celix_hash_map_t* map,
const char* strKey, long
visit = visit->next;
}
if (removedEntry != NULL) {
+char* removedKey = NULL;
+if (map->keyType == CELIX_HASH_MAP_STRING_KEY) {
+removedKey = (char*)removedEntry->key.strKey;
+}
celix_hashMap_destroyRemovedEntry(map, removedEntry);
+if (removedKey) {
+celix_hashMap_destroyRemovedKey(map, removedKey);
+}
return true;
}
return false;
}
-static void celix_hashMap_init(
+celix_status_t celix_hashMap_init(
celix_hash_map_t* map,
celix_hash_map_key_type_e keyType,
unsigned int initialCapacity,
double loadFactor,
unsigned int (*hashKeyFn)(const celix_hash_map_key_t*),
bool (*equalsKeyFn)(const celix_hash_map_key_t*, const
celix_hash_map_key_t*)) {
map->loadFactor = loadFactor;
Review Comment:
`loadFactor`, which is always less than 1, does not apply to separate
chaining hash table. It does apply to linear probing and double hashing
implementation, both of which have nothing to do with our implementation.
For separate chaining implementation, it is normal for a list to contain
about 5 or 10 keys. Our resize implementation will lead to too many
unnecessary resize.
For reference, I quote an exercise in Robert Sedgewick's Algorithms in C++:
> 14.45 Implement a version of Program 14.7 for separate chaining that
increases the table size by a factor of 10 each time the average list length is
equal to 10.
##
libs/utils/src/celix_hash_map.c:
##
@@ -237,11 +235,12 @@ static void celix_hashMap_addEntry(celix_hash_map_t* map,
unsigned int hash, con
newEntry->next = entry;
map->buckets[bucketIndex] = newEntry;
if (map->size++ >= celix_hashMap_threshold(map)) {
-celix_hashMap_resize(map, 2 * map->bucketsSize);
+return celix_hashMap_resize(map, 2 * map->bucketsSize);
Review Comment:
For weakly stored key, it will make huge difference.
##
libs/utils/src/celix_hash_map.c:
##
@@ -599,33 +675,49 @@ bool celix_longHashMapIterator_isEnd(const
celix_long_hash_map_iterator_t* iter)
void celix_stringHashMapIterator_next(celix_string_hash_map_iterator_t* iter) {
const celix_hash_map_t* map = iter->_internal[0];
celix_hash_map_entry_t *entry = iter->_internal[1];
+iter->index += 1;
Review Comment:
`iter.index` will be larger than `map->genericMap.size` even if it has
reached the end. Does this break some invariant?
##
libs/utils/src/celix_hash_map.c:
##
@@ -562,6 +591,33 @@ void celix_longHashMap_clear(celix_long_hash_map_t* map) {
celix_hashMap_clear(&map->genericMap);
}
+static bool celix_hashMap_equals(const celix_hash_map_t* map1, const
celix_hash_map_t* map2) {
+if (map1 == NULL && map2 == NULL) {
+return true;
+}
+if (map1 == NULL || map2 == NULL) {
+return false;
+}
+if (map1->size != map2->size) {
+return false;
+}
+for (celix_hash_map_entry_t* entry = celix_hashMap_firstEntry(map1); entry
!= NULL; entry = celix_hashMap_nextEntry(map1, entry)) {
+void* value = celix_hashMap_get(map2, entry->key.strKey,
entry->key.longKey);
+if (value == NULL || memcmp(&entry->value, value,
sizeof(entry->value)) != 0) {
Revi
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1382496338
##
libs/utils/include/celix_version.h:
##
@@ -20,6 +20,11 @@
#ifndef CELIX_CELIX_VERSION_H
#define CELIX_CELIX_VERSION_H
+#include
+#include
+
+#include "celix_utils_export.h"
Review Comment:
The above includes duplicate the following includes.
##
libs/utils/src/version.c:
##
@@ -95,15 +97,20 @@ celix_status_t version_isCompatible(version_pt user,
version_pt provider, bool*
return CELIX_SUCCESS;
}
-celix_version_t* celix_version_createVersion(int major, int minor, int micro,
const char* qualifier) {
+celix_version_t* celix_version_createVersion(int major, int minor, int micro,
const char * qualifier) {
+return celix_version_create(major, minor, micro, qualifier);
+}
+
Review Comment:
```suggestion
```
Not used any more.
##
libs/utils/include/celix_version.h:
##
@@ -77,27 +97,50 @@ CELIX_UTILS_EXPORT celix_version_t*
celix_version_copy(const celix_version_t* ve
*
* There must be no whitespace in version.
*
- * @param versionStr String representation of the version identifier.
+ * @param[in] versionStr String representation of the version identifier.
* @return The created version or NULL if the input was invalid.
*/
CELIX_UTILS_EXPORT celix_version_t*
celix_version_createVersionFromString(const char *versionStr);
/**
- * The empty version "0.0.0".
- *
+ * @brief Create empty version "0.0.0".
*/
CELIX_UTILS_EXPORT celix_version_t* celix_version_createEmptyVersion();
+/**
+ * @brief Gets the major version number of a celix version.
+ *
+ * @param[in] version The celix version.
+ * @return The major version number.
+ */
CELIX_UTILS_EXPORT int celix_version_getMajor(const celix_version_t* version);
+/**
+ * @brief Gets the minor version number of a celix version.
+ *
+ * @param[in] version The celix version.
+ * @return The minor version number.
+ */
CELIX_UTILS_EXPORT int celix_version_getMinor(const celix_version_t* version);
+/**
+ * @brief Gets the micro version number of a celix version.
+ *
+ * @param[in] version The celix version.
+ * @return The micro version number.
+ */
CELIX_UTILS_EXPORT int celix_version_getMicro(const celix_version_t* version);
+/**
+ * @brief Gets the version qualifier of a celix version.
+ *
+ * @param[in] version The celix version.
+ * @return The version qualifier, or NULL if no qualifier is present.
Review Comment:
Empty string if no qualifier is present?
##
libs/utils/src/celix_hash_map.c:
##
@@ -73,10 +61,11 @@ typedef struct celix_hash_map {
bool (*equalsKeyFunction)(const celix_hash_map_key_t* key1, const
celix_hash_map_key_t* key2);
void (*simpleRemovedCallback)(void* value);
void* removedCallbackData;
-void (*removedStringKeyCallback)(void* data, const char* removedKey,
celix_hash_map_value_t removedValue);
-void (*removedLongKeyCallback)(void* data, long removedKey,
celix_hash_map_value_t removedValue);
+void (*removedStringEntryCallback)(void* data, const char* removedKey,
celix_hash_map_value_t removedValue);
Review Comment:
Is `emptyValue` unused? If so, it should be removed.
##
libs/utils/src/version.c:
##
@@ -289,6 +301,15 @@ char* celix_version_toString(const celix_version_t*
version) {
return string;
}
+bool celix_version_fillString(const celix_version_t* version, char *str,
size_t strLen) {
Review Comment:
The return value of `asprintf` in `celix_version_toString` is not checked.
##
libs/utils/include/celix_version.h:
##
@@ -30,37 +35,52 @@ extern "C" {
#include "celix_utils_export.h"
/**
- * The definition of the celix_version_t* abstract data type.
+ * @file celix_version.h
+ * @brief Header file for the Celix Version API.
+ *
+ * The Celix Version API provides a means for storing and manipulating version
information, which consists of
+ * three non-negative integers for the major, minor, and micro version, and an
optional string for the qualifier.
+ * This implementation is based on the Semantic Versioning specification
(SemVer).
+ * Functions are provided for creating and destroying version objects,
comparing versions, and extracting the individual version components.
+ */
+
+/**
+ * @brief The definition of the celix_version_t* abstract data type.
*/
typedef struct celix_version celix_version_t;
/**
- * Creates a new celix_version_t* using the supplied arguments.
+ * @brief Create a new celix_version_t* using the supplied arguments.
*
- * @param major Major component of the version identifier.
- * @param minor Minor component of the version identifier.
- * @param micro Micro component of the version identifier.
- * @param qualifier Qualifier component of the version identifier. If
- *null is specified, then the qualifier will be set to
+ * @param[in] major Major component of the version identifier.
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1382405959
##
libs/utils/src/properties.c:
##
@@ -191,324 +476,423 @@ static void parseLine(const char* line,
celix_properties_t *props) {
}
if (!isComment) {
-//printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
+// printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
celix_properties_set(props, celix_utils_trimInPlace(key),
celix_utils_trimInPlace(value));
}
-if(key) {
+if (key) {
free(key);
-}
-if(value) {
free(value);
}
-
}
+celix_properties_t* celix_properties_loadWithStream(FILE* file) {
+if (file == NULL) {
+return NULL;
+}
-/**
-
**
- * Updated API
-
**
-
**/
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+if (!props) {
+celix_err_push("Failed to create properties");
+return NULL;
+}
+int rc = fseek(file, 0, SEEK_END);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to end of file. Got error %i", errno);
+return NULL;
+}
+size_t fileSize = ftell(file);
+rc = fseek(file, 0, SEEK_SET);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to start of file. Got error %i", errno);
+return NULL;
+}
+char* fileBuffer = malloc(fileSize + 1);
+if (fileBuffer == NULL) {
+celix_err_pushf("Cannot allocate memory for file buffer. Got error
%i", errno);
+return NULL;
+}
-celix_properties_t* celix_properties_create(void) {
-return hashMap_create(utils_stringHash, utils_stringHash,
utils_stringEquals, utils_stringEquals);
-}
+size_t rs = fread(fileBuffer, sizeof(char), fileSize, file);
+if (rs < fileSize) {
+fprintf(stderr, "fread read only %zu bytes out of %zu\n", rs,
fileSize);
+}
+fileBuffer[fileSize] = '\0'; // ensure a '\0' at the end of the fileBuffer
-void celix_properties_destroy(celix_properties_t *properties) {
-if (properties != NULL) {
-hash_map_iterator_pt iter = hashMapIterator_create(properties);
-while (hashMapIterator_hasNext(iter)) {
-hash_map_entry_pt entry = hashMapIterator_nextEntry(iter);
-hashMapEntry_clear(entry, true, true);
-}
-hashMapIterator_destroy(iter);
-hashMap_destroy(properties, false, false);
+char* savePtr = NULL;
+char* line = strtok_r(fileBuffer, "\n", &savePtr);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &savePtr);
}
+free(fileBuffer);
+
+return celix_steal_ptr(props);
}
-celix_properties_t* celix_properties_load(const char *filename) {
-FILE *file = fopen(filename, "r");
-if (file == NULL) {
+celix_properties_t* celix_properties_loadFromString(const char* input) {
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+celix_autofree char* in = celix_utils_strdup(input);
+if (!props || !in) {
+celix_err_push("Failed to create properties or duplicate input
string");
return NULL;
}
-celix_properties_t *props = celix_properties_loadWithStream(file);
-fclose(file);
-return props;
-}
-
-celix_properties_t* celix_properties_loadWithStream(FILE *file) {
-celix_properties_t *props = NULL;
-if (file != NULL ) {
-char *saveptr;
-char *filebuffer = NULL;
-char *line = NULL;
-ssize_t file_size = 0;
-
-props = celix_properties_create();
-fseek(file, 0, SEEK_END);
-file_size = ftell(file);
-fseek(file, 0, SEEK_SET);
+char* line = NULL;
+char* saveLinePointer = NULL;
+line = strtok_r(in, "\n", &saveLinePointer);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &saveLinePointer);
+}
+return celix_steal_ptr(props);
+}
-if (file_size > 0) {
-filebuffer = calloc(file_size + 1, sizeof(char));
-if (filebuffer) {
-size_t rs = fread(filebuffer, sizeof(char), file_size, file);
-if (rs != file_size) {
-fprintf(stderr,"fread read only %lu bytes out of %lu\n",
(long unsigned int) rs, (long unsigned int) file_size);
-}
-filebuffer[file_size]='\0'
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1382404373
##
libs/utils/include/celix_properties.h:
##
@@ -17,86 +17,474 @@
* under the License.
*/
-#ifndef CELIX_PROPERTIES_H_
-#define CELIX_PROPERTIES_H_
+/**
+ * @file celix_properties.h
+ * @brief Header file for the Celix Properties API.
+ *
+ * The Celix Properties API provides a means for storing and manipulating
key-value pairs, called properties,
+ * which can be used to store configuration data or metadata for a service,
component, or framework configuration.
+ * Functions are provided for creating and destroying property sets, loading
and storing properties from/to a file
+ * or stream, and setting, getting, and unsetting individual properties. There
are also functions for converting
+ * property values to various types (e.g. long, bool, double) and for
iterating over the properties in a set.
+ *
+ * Supported property value types include:
+ * - string (char*)
+ * - long
+ * - double
+ * - bool
+ * - celix_version_t*
+ */
#include
-#include
#include "celix_cleanup.h"
#include "celix_compiler.h"
#include "celix_errno.h"
#include "celix_utils_export.h"
+#include "celix_version.h"
+
+#ifndef CELIX_PROPERTIES_H_
+#define CELIX_PROPERTIES_H_
#ifdef __cplusplus
extern "C" {
#endif
-typedef struct hashMap celix_properties_t; //opaque struct, TODO try to make
this a celix_properties struct
+/**
+ * @brief celix_properties_t is a type that represents a set of key-value
pairs called properties.
+ *
+ * @note Not thread safe.
+ */
+typedef struct celix_properties celix_properties_t;
+
+/**
+ * @brief Enum representing the possible types of a property value.
+ */
+typedef enum celix_properties_value_type {
+CELIX_PROPERTIES_VALUE_TYPE_UNSET = 0, /**< Property value is not set. */
+CELIX_PROPERTIES_VALUE_TYPE_STRING = 1, /**< Property value is a string. */
+CELIX_PROPERTIES_VALUE_TYPE_LONG = 2, /**< Property value is a long
integer. */
+CELIX_PROPERTIES_VALUE_TYPE_DOUBLE = 3, /**< Property value is a double. */
+CELIX_PROPERTIES_VALUE_TYPE_BOOL = 4, /**< Property value is a boolean.
*/
+CELIX_PROPERTIES_VALUE_TYPE_VERSION = 5 /**< Property value is a Celix
version. */
+} celix_properties_value_type_e;
+
+/**
+ * @brief A structure representing a single value entry in a property set.
+ */
+typedef struct celix_properties_entry {
+const char* value; /**< The string value or string
representation of a non-string
+ typed value.*/
+celix_properties_value_type_e valueType; /**< The type of the value of the
entry */
+union {
+const char* strValue;/**< The string value of the
entry. */
+long longValue; /**< The long integer value of
the entry. */
+double doubleValue; /**< The double-precision
floating point value of the entry. */
+bool boolValue; /**< The boolean value of the
entry. */
+const celix_version_t* versionValue; /**< The Celix version value of
the entry. */
+} typed; /**< The typed values of the
entry. Only valid if valueType
+ is not
CELIX_PROPERTIES_VALUE_TYPE_UNSET and only the matching
+ value types should be used.
E.g typed.boolValue if valueType is
+
CELIX_PROPERTIES_VALUE_TYPE_BOOL. */
+} celix_properties_entry_t;
+
+/**
+ * @brief Represents an iterator for iterating over the entries in a
celix_properties_t object.
+ */
typedef struct celix_properties_iterator {
-//private data
-void* _data1;
-void* _data2;
-void* _data3;
-int _data4;
-int _data5;
-} celix_properties_iterator_t;
+/**
+ * @brief The index of the current iterator.
+ */
+size_t index;
+/**
+ * @brief Te current key.
+ */
+const char* key;
-/**
-
**
- * Updated API
-
**
-
**/
+/**
+ * @brief The current value entry.
+ */
+celix_properties_entry_t entry;
-CELIX_UTILS_EXPORT celix_properties_t* celix_properties_create(void);
+/**
+ * @brief Private data used to implement the iterator.
+ */
+char _data[56];
+} celix_properties_iterator_t;
-CELIX_UTILS_EXPORT void celix_properties_destroy(celix_properties_t
*pro
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1382402803
##
libs/utils/include/celix_properties.h:
##
@@ -17,86 +17,474 @@
* under the License.
*/
-#ifndef CELIX_PROPERTIES_H_
-#define CELIX_PROPERTIES_H_
+/**
+ * @file celix_properties.h
+ * @brief Header file for the Celix Properties API.
+ *
+ * The Celix Properties API provides a means for storing and manipulating
key-value pairs, called properties,
+ * which can be used to store configuration data or metadata for a service,
component, or framework configuration.
+ * Functions are provided for creating and destroying property sets, loading
and storing properties from/to a file
+ * or stream, and setting, getting, and unsetting individual properties. There
are also functions for converting
+ * property values to various types (e.g. long, bool, double) and for
iterating over the properties in a set.
+ *
+ * Supported property value types include:
+ * - string (char*)
+ * - long
+ * - double
+ * - bool
+ * - celix_version_t*
+ */
#include
-#include
#include "celix_cleanup.h"
#include "celix_compiler.h"
#include "celix_errno.h"
#include "celix_utils_export.h"
+#include "celix_version.h"
+
+#ifndef CELIX_PROPERTIES_H_
+#define CELIX_PROPERTIES_H_
#ifdef __cplusplus
extern "C" {
#endif
-typedef struct hashMap celix_properties_t; //opaque struct, TODO try to make
this a celix_properties struct
+/**
+ * @brief celix_properties_t is a type that represents a set of key-value
pairs called properties.
+ *
+ * @note Not thread safe.
+ */
+typedef struct celix_properties celix_properties_t;
+
+/**
+ * @brief Enum representing the possible types of a property value.
+ */
+typedef enum celix_properties_value_type {
+CELIX_PROPERTIES_VALUE_TYPE_UNSET = 0, /**< Property value is not set. */
+CELIX_PROPERTIES_VALUE_TYPE_STRING = 1, /**< Property value is a string. */
+CELIX_PROPERTIES_VALUE_TYPE_LONG = 2, /**< Property value is a long
integer. */
+CELIX_PROPERTIES_VALUE_TYPE_DOUBLE = 3, /**< Property value is a double. */
+CELIX_PROPERTIES_VALUE_TYPE_BOOL = 4, /**< Property value is a boolean.
*/
+CELIX_PROPERTIES_VALUE_TYPE_VERSION = 5 /**< Property value is a Celix
version. */
+} celix_properties_value_type_e;
+
+/**
+ * @brief A structure representing a single value entry in a property set.
+ */
+typedef struct celix_properties_entry {
+const char* value; /**< The string value or string
representation of a non-string
+ typed value.*/
+celix_properties_value_type_e valueType; /**< The type of the value of the
entry */
+union {
+const char* strValue;/**< The string value of the
entry. */
+long longValue; /**< The long integer value of
the entry. */
+double doubleValue; /**< The double-precision
floating point value of the entry. */
+bool boolValue; /**< The boolean value of the
entry. */
+const celix_version_t* versionValue; /**< The Celix version value of
the entry. */
+} typed; /**< The typed values of the
entry. Only valid if valueType
+ is not
CELIX_PROPERTIES_VALUE_TYPE_UNSET and only the matching
+ value types should be used.
E.g typed.boolValue if valueType is
+
CELIX_PROPERTIES_VALUE_TYPE_BOOL. */
+} celix_properties_entry_t;
+
+/**
+ * @brief Represents an iterator for iterating over the entries in a
celix_properties_t object.
+ */
typedef struct celix_properties_iterator {
-//private data
-void* _data1;
-void* _data2;
-void* _data3;
-int _data4;
-int _data5;
-} celix_properties_iterator_t;
+/**
+ * @brief The index of the current iterator.
+ */
+size_t index;
+/**
+ * @brief Te current key.
+ */
+const char* key;
-/**
-
**
- * Updated API
-
**
-
**/
+/**
+ * @brief The current value entry.
+ */
+celix_properties_entry_t entry;
-CELIX_UTILS_EXPORT celix_properties_t* celix_properties_create(void);
+/**
+ * @brief Private data used to implement the iterator.
+ */
+char _data[56];
+} celix_properties_iterator_t;
-CELIX_UTILS_EXPORT void celix_properties_destroy(celix_properties_t
*pro
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1382402803
##
libs/utils/include/celix_properties.h:
##
@@ -17,86 +17,474 @@
* under the License.
*/
-#ifndef CELIX_PROPERTIES_H_
-#define CELIX_PROPERTIES_H_
+/**
+ * @file celix_properties.h
+ * @brief Header file for the Celix Properties API.
+ *
+ * The Celix Properties API provides a means for storing and manipulating
key-value pairs, called properties,
+ * which can be used to store configuration data or metadata for a service,
component, or framework configuration.
+ * Functions are provided for creating and destroying property sets, loading
and storing properties from/to a file
+ * or stream, and setting, getting, and unsetting individual properties. There
are also functions for converting
+ * property values to various types (e.g. long, bool, double) and for
iterating over the properties in a set.
+ *
+ * Supported property value types include:
+ * - string (char*)
+ * - long
+ * - double
+ * - bool
+ * - celix_version_t*
+ */
#include
-#include
#include "celix_cleanup.h"
#include "celix_compiler.h"
#include "celix_errno.h"
#include "celix_utils_export.h"
+#include "celix_version.h"
+
+#ifndef CELIX_PROPERTIES_H_
+#define CELIX_PROPERTIES_H_
#ifdef __cplusplus
extern "C" {
#endif
-typedef struct hashMap celix_properties_t; //opaque struct, TODO try to make
this a celix_properties struct
+/**
+ * @brief celix_properties_t is a type that represents a set of key-value
pairs called properties.
+ *
+ * @note Not thread safe.
+ */
+typedef struct celix_properties celix_properties_t;
+
+/**
+ * @brief Enum representing the possible types of a property value.
+ */
+typedef enum celix_properties_value_type {
+CELIX_PROPERTIES_VALUE_TYPE_UNSET = 0, /**< Property value is not set. */
+CELIX_PROPERTIES_VALUE_TYPE_STRING = 1, /**< Property value is a string. */
+CELIX_PROPERTIES_VALUE_TYPE_LONG = 2, /**< Property value is a long
integer. */
+CELIX_PROPERTIES_VALUE_TYPE_DOUBLE = 3, /**< Property value is a double. */
+CELIX_PROPERTIES_VALUE_TYPE_BOOL = 4, /**< Property value is a boolean.
*/
+CELIX_PROPERTIES_VALUE_TYPE_VERSION = 5 /**< Property value is a Celix
version. */
+} celix_properties_value_type_e;
+
+/**
+ * @brief A structure representing a single value entry in a property set.
+ */
+typedef struct celix_properties_entry {
+const char* value; /**< The string value or string
representation of a non-string
+ typed value.*/
+celix_properties_value_type_e valueType; /**< The type of the value of the
entry */
+union {
+const char* strValue;/**< The string value of the
entry. */
+long longValue; /**< The long integer value of
the entry. */
+double doubleValue; /**< The double-precision
floating point value of the entry. */
+bool boolValue; /**< The boolean value of the
entry. */
+const celix_version_t* versionValue; /**< The Celix version value of
the entry. */
+} typed; /**< The typed values of the
entry. Only valid if valueType
+ is not
CELIX_PROPERTIES_VALUE_TYPE_UNSET and only the matching
+ value types should be used.
E.g typed.boolValue if valueType is
+
CELIX_PROPERTIES_VALUE_TYPE_BOOL. */
+} celix_properties_entry_t;
+
+/**
+ * @brief Represents an iterator for iterating over the entries in a
celix_properties_t object.
+ */
typedef struct celix_properties_iterator {
-//private data
-void* _data1;
-void* _data2;
-void* _data3;
-int _data4;
-int _data5;
-} celix_properties_iterator_t;
+/**
+ * @brief The index of the current iterator.
+ */
+size_t index;
+/**
+ * @brief Te current key.
+ */
+const char* key;
-/**
-
**
- * Updated API
-
**
-
**/
+/**
+ * @brief The current value entry.
+ */
+celix_properties_entry_t entry;
-CELIX_UTILS_EXPORT celix_properties_t* celix_properties_create(void);
+/**
+ * @brief Private data used to implement the iterator.
+ */
+char _data[56];
+} celix_properties_iterator_t;
-CELIX_UTILS_EXPORT void celix_properties_destroy(celix_properties_t
*pro
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1382402803
##
libs/utils/include/celix_properties.h:
##
@@ -17,86 +17,474 @@
* under the License.
*/
-#ifndef CELIX_PROPERTIES_H_
-#define CELIX_PROPERTIES_H_
+/**
+ * @file celix_properties.h
+ * @brief Header file for the Celix Properties API.
+ *
+ * The Celix Properties API provides a means for storing and manipulating
key-value pairs, called properties,
+ * which can be used to store configuration data or metadata for a service,
component, or framework configuration.
+ * Functions are provided for creating and destroying property sets, loading
and storing properties from/to a file
+ * or stream, and setting, getting, and unsetting individual properties. There
are also functions for converting
+ * property values to various types (e.g. long, bool, double) and for
iterating over the properties in a set.
+ *
+ * Supported property value types include:
+ * - string (char*)
+ * - long
+ * - double
+ * - bool
+ * - celix_version_t*
+ */
#include
-#include
#include "celix_cleanup.h"
#include "celix_compiler.h"
#include "celix_errno.h"
#include "celix_utils_export.h"
+#include "celix_version.h"
+
+#ifndef CELIX_PROPERTIES_H_
+#define CELIX_PROPERTIES_H_
#ifdef __cplusplus
extern "C" {
#endif
-typedef struct hashMap celix_properties_t; //opaque struct, TODO try to make
this a celix_properties struct
+/**
+ * @brief celix_properties_t is a type that represents a set of key-value
pairs called properties.
+ *
+ * @note Not thread safe.
+ */
+typedef struct celix_properties celix_properties_t;
+
+/**
+ * @brief Enum representing the possible types of a property value.
+ */
+typedef enum celix_properties_value_type {
+CELIX_PROPERTIES_VALUE_TYPE_UNSET = 0, /**< Property value is not set. */
+CELIX_PROPERTIES_VALUE_TYPE_STRING = 1, /**< Property value is a string. */
+CELIX_PROPERTIES_VALUE_TYPE_LONG = 2, /**< Property value is a long
integer. */
+CELIX_PROPERTIES_VALUE_TYPE_DOUBLE = 3, /**< Property value is a double. */
+CELIX_PROPERTIES_VALUE_TYPE_BOOL = 4, /**< Property value is a boolean.
*/
+CELIX_PROPERTIES_VALUE_TYPE_VERSION = 5 /**< Property value is a Celix
version. */
+} celix_properties_value_type_e;
+
+/**
+ * @brief A structure representing a single value entry in a property set.
+ */
+typedef struct celix_properties_entry {
+const char* value; /**< The string value or string
representation of a non-string
+ typed value.*/
+celix_properties_value_type_e valueType; /**< The type of the value of the
entry */
+union {
+const char* strValue;/**< The string value of the
entry. */
+long longValue; /**< The long integer value of
the entry. */
+double doubleValue; /**< The double-precision
floating point value of the entry. */
+bool boolValue; /**< The boolean value of the
entry. */
+const celix_version_t* versionValue; /**< The Celix version value of
the entry. */
+} typed; /**< The typed values of the
entry. Only valid if valueType
+ is not
CELIX_PROPERTIES_VALUE_TYPE_UNSET and only the matching
+ value types should be used.
E.g typed.boolValue if valueType is
+
CELIX_PROPERTIES_VALUE_TYPE_BOOL. */
+} celix_properties_entry_t;
+
+/**
+ * @brief Represents an iterator for iterating over the entries in a
celix_properties_t object.
+ */
typedef struct celix_properties_iterator {
-//private data
-void* _data1;
-void* _data2;
-void* _data3;
-int _data4;
-int _data5;
-} celix_properties_iterator_t;
+/**
+ * @brief The index of the current iterator.
+ */
+size_t index;
+/**
+ * @brief Te current key.
+ */
+const char* key;
-/**
-
**
- * Updated API
-
**
-
**/
+/**
+ * @brief The current value entry.
+ */
+celix_properties_entry_t entry;
-CELIX_UTILS_EXPORT celix_properties_t* celix_properties_create(void);
+/**
+ * @brief Private data used to implement the iterator.
+ */
+char _data[56];
+} celix_properties_iterator_t;
-CELIX_UTILS_EXPORT void celix_properties_destroy(celix_properties_t
*pro
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1382398863
##
libs/utils/include/celix_properties.h:
##
@@ -17,86 +17,474 @@
* under the License.
*/
-#ifndef CELIX_PROPERTIES_H_
-#define CELIX_PROPERTIES_H_
+/**
+ * @file celix_properties.h
+ * @brief Header file for the Celix Properties API.
+ *
+ * The Celix Properties API provides a means for storing and manipulating
key-value pairs, called properties,
+ * which can be used to store configuration data or metadata for a service,
component, or framework configuration.
+ * Functions are provided for creating and destroying property sets, loading
and storing properties from/to a file
+ * or stream, and setting, getting, and unsetting individual properties. There
are also functions for converting
+ * property values to various types (e.g. long, bool, double) and for
iterating over the properties in a set.
+ *
+ * Supported property value types include:
+ * - string (char*)
+ * - long
+ * - double
+ * - bool
+ * - celix_version_t*
+ */
#include
-#include
#include "celix_cleanup.h"
#include "celix_compiler.h"
#include "celix_errno.h"
#include "celix_utils_export.h"
+#include "celix_version.h"
+
+#ifndef CELIX_PROPERTIES_H_
+#define CELIX_PROPERTIES_H_
#ifdef __cplusplus
extern "C" {
#endif
-typedef struct hashMap celix_properties_t; //opaque struct, TODO try to make
this a celix_properties struct
+/**
+ * @brief celix_properties_t is a type that represents a set of key-value
pairs called properties.
+ *
+ * @note Not thread safe.
+ */
+typedef struct celix_properties celix_properties_t;
+
+/**
+ * @brief Enum representing the possible types of a property value.
+ */
+typedef enum celix_properties_value_type {
+CELIX_PROPERTIES_VALUE_TYPE_UNSET = 0, /**< Property value is not set. */
+CELIX_PROPERTIES_VALUE_TYPE_STRING = 1, /**< Property value is a string. */
+CELIX_PROPERTIES_VALUE_TYPE_LONG = 2, /**< Property value is a long
integer. */
+CELIX_PROPERTIES_VALUE_TYPE_DOUBLE = 3, /**< Property value is a double. */
+CELIX_PROPERTIES_VALUE_TYPE_BOOL = 4, /**< Property value is a boolean.
*/
+CELIX_PROPERTIES_VALUE_TYPE_VERSION = 5 /**< Property value is a Celix
version. */
+} celix_properties_value_type_e;
+
+/**
+ * @brief A structure representing a single value entry in a property set.
+ */
+typedef struct celix_properties_entry {
+const char* value; /**< The string value or string
representation of a non-string
+ typed value.*/
+celix_properties_value_type_e valueType; /**< The type of the value of the
entry */
+union {
+const char* strValue;/**< The string value of the
entry. */
+long longValue; /**< The long integer value of
the entry. */
+double doubleValue; /**< The double-precision
floating point value of the entry. */
+bool boolValue; /**< The boolean value of the
entry. */
+const celix_version_t* versionValue; /**< The Celix version value of
the entry. */
+} typed; /**< The typed values of the
entry. Only valid if valueType
+ is not
CELIX_PROPERTIES_VALUE_TYPE_UNSET and only the matching
+ value types should be used.
E.g typed.boolValue if valueType is
+
CELIX_PROPERTIES_VALUE_TYPE_BOOL. */
+} celix_properties_entry_t;
+
+/**
+ * @brief Represents an iterator for iterating over the entries in a
celix_properties_t object.
+ */
typedef struct celix_properties_iterator {
-//private data
-void* _data1;
-void* _data2;
-void* _data3;
-int _data4;
-int _data5;
-} celix_properties_iterator_t;
+/**
+ * @brief The index of the current iterator.
+ */
+size_t index;
+/**
+ * @brief Te current key.
+ */
+const char* key;
-/**
-
**
- * Updated API
-
**
-
**/
+/**
+ * @brief The current value entry.
+ */
+celix_properties_entry_t entry;
-CELIX_UTILS_EXPORT celix_properties_t* celix_properties_create(void);
+/**
+ * @brief Private data used to implement the iterator.
+ */
+char _data[56];
+} celix_properties_iterator_t;
-CELIX_UTILS_EXPORT void celix_properties_destroy(celix_properties_t
*pro
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1382392394
##
libs/utils/include/celix_properties.h:
##
@@ -17,86 +17,474 @@
* under the License.
*/
-#ifndef CELIX_PROPERTIES_H_
-#define CELIX_PROPERTIES_H_
+/**
+ * @file celix_properties.h
+ * @brief Header file for the Celix Properties API.
+ *
+ * The Celix Properties API provides a means for storing and manipulating
key-value pairs, called properties,
+ * which can be used to store configuration data or metadata for a service,
component, or framework configuration.
+ * Functions are provided for creating and destroying property sets, loading
and storing properties from/to a file
+ * or stream, and setting, getting, and unsetting individual properties. There
are also functions for converting
+ * property values to various types (e.g. long, bool, double) and for
iterating over the properties in a set.
+ *
+ * Supported property value types include:
+ * - string (char*)
+ * - long
+ * - double
+ * - bool
+ * - celix_version_t*
+ */
#include
-#include
#include "celix_cleanup.h"
#include "celix_compiler.h"
#include "celix_errno.h"
#include "celix_utils_export.h"
+#include "celix_version.h"
+
+#ifndef CELIX_PROPERTIES_H_
+#define CELIX_PROPERTIES_H_
#ifdef __cplusplus
extern "C" {
#endif
-typedef struct hashMap celix_properties_t; //opaque struct, TODO try to make
this a celix_properties struct
+/**
+ * @brief celix_properties_t is a type that represents a set of key-value
pairs called properties.
+ *
+ * @note Not thread safe.
+ */
+typedef struct celix_properties celix_properties_t;
+
+/**
+ * @brief Enum representing the possible types of a property value.
+ */
+typedef enum celix_properties_value_type {
+CELIX_PROPERTIES_VALUE_TYPE_UNSET = 0, /**< Property value is not set. */
+CELIX_PROPERTIES_VALUE_TYPE_STRING = 1, /**< Property value is a string. */
+CELIX_PROPERTIES_VALUE_TYPE_LONG = 2, /**< Property value is a long
integer. */
+CELIX_PROPERTIES_VALUE_TYPE_DOUBLE = 3, /**< Property value is a double. */
+CELIX_PROPERTIES_VALUE_TYPE_BOOL = 4, /**< Property value is a boolean.
*/
+CELIX_PROPERTIES_VALUE_TYPE_VERSION = 5 /**< Property value is a Celix
version. */
+} celix_properties_value_type_e;
+
+/**
+ * @brief A structure representing a single value entry in a property set.
+ */
+typedef struct celix_properties_entry {
+const char* value; /**< The string value or string
representation of a non-string
+ typed value.*/
+celix_properties_value_type_e valueType; /**< The type of the value of the
entry */
+union {
+const char* strValue;/**< The string value of the
entry. */
+long longValue; /**< The long integer value of
the entry. */
+double doubleValue; /**< The double-precision
floating point value of the entry. */
+bool boolValue; /**< The boolean value of the
entry. */
+const celix_version_t* versionValue; /**< The Celix version value of
the entry. */
+} typed; /**< The typed values of the
entry. Only valid if valueType
+ is not
CELIX_PROPERTIES_VALUE_TYPE_UNSET and only the matching
+ value types should be used.
E.g typed.boolValue if valueType is
+
CELIX_PROPERTIES_VALUE_TYPE_BOOL. */
+} celix_properties_entry_t;
+
+/**
+ * @brief Represents an iterator for iterating over the entries in a
celix_properties_t object.
+ */
typedef struct celix_properties_iterator {
-//private data
-void* _data1;
-void* _data2;
-void* _data3;
-int _data4;
-int _data5;
-} celix_properties_iterator_t;
+/**
+ * @brief The index of the current iterator.
+ */
+size_t index;
+/**
+ * @brief Te current key.
Review Comment:
```suggestion
* @brief The current key.
```
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1380068055
##
libs/utils/include/celix/Properties.h:
##
@@ -145,20 +172,19 @@ namespace celix {
};
-Properties() : cProps{celix_properties_create(),
[](celix_properties_t* p) { celix_properties_destroy(p); }} {}
+Properties() : cProps{createCProps(celix_properties_create())} {}
Properties(Properties&&) = default;
Review Comment:
After move, the stored pointer in `cProps` is nullptr? That will break class
invariant and lead to crash.
##
libs/utils/src/properties.c:
##
@@ -103,12 +132,275 @@ static void updateBuffers(char **key, char ** value,
char **output, int outputPo
}
}
-static void parseLine(const char* line, celix_properties_t *props) {
-int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strup or storing
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
+if (str == NULL) {
+return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+}
+size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+size_t left = CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
+char* result;
+if (len < left) {
+
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
+result =
&properties->stringBuffer[properties->currentStringBufferIndex];
+properties->currentStringBufferIndex += (int)len;
+} else {
+result = celix_utils_strdup(str);
+}
+return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char*
str) {
+if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str ==
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+// str is static const char* const -> nop
+} else if (str >= properties->stringBuffer &&
+ str < (properties->stringBuffer +
CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+// str is part of the properties string buffer -> nop
+} else {
+free(str);
+}
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+static celix_status_t celix_properties_fillEntry(celix_properties_t*
properties,
+ celix_properties_entry_t*
entry,
+ const char** strValue,
+ const long* longValue,
+ const double* doubleValue,
+ const bool* boolValue,
+ celix_version_t*
versionValue) {
+char convertedValueBuffer[32];
+if (strValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING;
+entry->value = celix_properties_createString(properties, *strValue);
+entry->typed.strValue = entry->value;
+} else if (longValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG;
+entry->typed.longValue = *longValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%li", entry->typed.longValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%li", entry->typed.longValue);
+entry->value = val;
+}
+} else if (doubleValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE;
+entry->typed.doubleValue = *doubleValue;
+int written = snprintf(convertedValueBuffer,
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+if (written < 0 || written >= sizeof(convertedValueBuffer)) {
+entry->value = celix_properties_createString(properties,
convertedValueBuffer);
+} else {
+char* val = NULL;
+asprintf(&val, "%f", entry->typed.doubleValue);
+entry->value = val;
+}
+} else if (boolValue != NULL) {
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL;
+entry->typed.boolValue = *boolValue;
+entry->value = entry->typed.boolValue ?
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+} else /*versionValue*/ {
+assert(versionValue != NULL);
+entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
+entry->typed.versionValue = versionValue;
+bool written = celix_version_fillString(versionValue
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1378791024
##
libs/utils/src/properties.c:
##
@@ -191,324 +476,423 @@ static void parseLine(const char* line,
celix_properties_t *props) {
}
if (!isComment) {
-//printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
+// printf("putting 'key'/'value' '%s'/'%s' in properties\n",
utils_stringTrim(key), utils_stringTrim(value));
celix_properties_set(props, celix_utils_trimInPlace(key),
celix_utils_trimInPlace(value));
}
-if(key) {
+if (key) {
free(key);
-}
-if(value) {
free(value);
}
-
}
+celix_properties_t* celix_properties_loadWithStream(FILE* file) {
+if (file == NULL) {
+return NULL;
+}
-/**
-
**
- * Updated API
-
**
-
**/
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+if (!props) {
+celix_err_push("Failed to create properties");
+return NULL;
+}
+int rc = fseek(file, 0, SEEK_END);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to end of file. Got error %i", errno);
+return NULL;
+}
+size_t fileSize = ftell(file);
+rc = fseek(file, 0, SEEK_SET);
+if (rc != 0) {
+celix_err_pushf("Cannot seek to start of file. Got error %i", errno);
+return NULL;
+}
+char* fileBuffer = malloc(fileSize + 1);
+if (fileBuffer == NULL) {
+celix_err_pushf("Cannot allocate memory for file buffer. Got error
%i", errno);
+return NULL;
+}
-celix_properties_t* celix_properties_create(void) {
-return hashMap_create(utils_stringHash, utils_stringHash,
utils_stringEquals, utils_stringEquals);
-}
+size_t rs = fread(fileBuffer, sizeof(char), fileSize, file);
+if (rs < fileSize) {
+fprintf(stderr, "fread read only %zu bytes out of %zu\n", rs,
fileSize);
+}
+fileBuffer[fileSize] = '\0'; // ensure a '\0' at the end of the fileBuffer
-void celix_properties_destroy(celix_properties_t *properties) {
-if (properties != NULL) {
-hash_map_iterator_pt iter = hashMapIterator_create(properties);
-while (hashMapIterator_hasNext(iter)) {
-hash_map_entry_pt entry = hashMapIterator_nextEntry(iter);
-hashMapEntry_clear(entry, true, true);
-}
-hashMapIterator_destroy(iter);
-hashMap_destroy(properties, false, false);
+char* savePtr = NULL;
+char* line = strtok_r(fileBuffer, "\n", &savePtr);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &savePtr);
}
+free(fileBuffer);
+
+return celix_steal_ptr(props);
}
-celix_properties_t* celix_properties_load(const char *filename) {
-FILE *file = fopen(filename, "r");
-if (file == NULL) {
+celix_properties_t* celix_properties_loadFromString(const char* input) {
+celix_autoptr(celix_properties_t) props = celix_properties_create();
+celix_autofree char* in = celix_utils_strdup(input);
+if (!props || !in) {
+celix_err_push("Failed to create properties or duplicate input
string");
return NULL;
}
-celix_properties_t *props = celix_properties_loadWithStream(file);
-fclose(file);
-return props;
-}
-
-celix_properties_t* celix_properties_loadWithStream(FILE *file) {
-celix_properties_t *props = NULL;
-if (file != NULL ) {
-char *saveptr;
-char *filebuffer = NULL;
-char *line = NULL;
-ssize_t file_size = 0;
-
-props = celix_properties_create();
-fseek(file, 0, SEEK_END);
-file_size = ftell(file);
-fseek(file, 0, SEEK_SET);
+char* line = NULL;
+char* saveLinePointer = NULL;
+line = strtok_r(in, "\n", &saveLinePointer);
+while (line != NULL) {
+celix_properties_parseLine(line, props);
+line = strtok_r(NULL, "\n", &saveLinePointer);
+}
+return celix_steal_ptr(props);
+}
-if (file_size > 0) {
-filebuffer = calloc(file_size + 1, sizeof(char));
-if (filebuffer) {
-size_t rs = fread(filebuffer, sizeof(char), file_size, file);
-if (rs != file_size) {
-fprintf(stderr,"fread read only %lu bytes out of %lu\n",
(long unsigned int) rs, (long unsigned int) file_size);
-}
-filebuffer[file_size]='\0'
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1378679080
##
libs/utils/gtest/CMakeLists.txt:
##
@@ -43,7 +43,7 @@ celix_deprecated_utils_headers(test_utils)
configure_file(resources/properties.txt
${CMAKE_CURRENT_BINARY_DIR}/resources-test/properties.txt COPYONLY)
if (CELIX_CXX17)
-add_library(test_utils_cxx17tests STATIC
+add_library(test_utils_cxx17tests OBJECT
Review Comment:
~~I have not figured out why static library does not work.~~
All unresolved symbols of the executable `test_utils` are satisfied by other
libraries, and it does not use any symbols in `test_utils_cxx17tests`.
Therefore, `test_utils_cxx17tests` is totally ignored by linker.
That is one subtlety of using static library.
Changing `test_utils_cxx17tests` to object library essentially makes it part
of `test_utils`.
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1378679080
##
libs/utils/gtest/CMakeLists.txt:
##
@@ -43,7 +43,7 @@ celix_deprecated_utils_headers(test_utils)
configure_file(resources/properties.txt
${CMAKE_CURRENT_BINARY_DIR}/resources-test/properties.txt COPYONLY)
if (CELIX_CXX17)
-add_library(test_utils_cxx17tests STATIC
+add_library(test_utils_cxx17tests OBJECT
Review Comment:
~~I have not figured out why static library does not work.~~
All unresolved symbols of the executable `test_utils` are satisfied by other
libraries, and it does not use any symbols in `test_utils_cxx17tests`.
Therefore, `test_utils_cxx17tests` is totally ignored.
Changing `test_utils_cxx17tests` to object library essentially makes it part
of `test_utils`.
##
libs/utils/gtest/CMakeLists.txt:
##
@@ -43,7 +43,7 @@ celix_deprecated_utils_headers(test_utils)
configure_file(resources/properties.txt
${CMAKE_CURRENT_BINARY_DIR}/resources-test/properties.txt COPYONLY)
if (CELIX_CXX17)
-add_library(test_utils_cxx17tests STATIC
+add_library(test_utils_cxx17tests OBJECT
Review Comment:
~~I have not figured out why static library does not work.~~
All unresolved symbols of the executable `test_utils` are satisfied by other
libraries, and it does not use any symbols in `test_utils_cxx17tests`.
Therefore, `test_utils_cxx17tests` is totally ignored by linker.
Changing `test_utils_cxx17tests` to object library essentially makes it part
of `test_utils`.
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1378679080
##
libs/utils/gtest/CMakeLists.txt:
##
@@ -43,7 +43,7 @@ celix_deprecated_utils_headers(test_utils)
configure_file(resources/properties.txt
${CMAKE_CURRENT_BINARY_DIR}/resources-test/properties.txt COPYONLY)
if (CELIX_CXX17)
-add_library(test_utils_cxx17tests STATIC
+add_library(test_utils_cxx17tests OBJECT
Review Comment:
~~I have not figured out why static library does not work.~~
All unresolved symbols of the executable `test_utils` are satisfied by other
library, and it does not use any symbols in `test_utils_cxx17tests`. Therefore,
`test_utils_cxx17tests` is totally ignored.
Changing `test_utils_cxx17tests` to object library essentially makes it part
of `test_utils`.
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1378679080
##
libs/utils/gtest/CMakeLists.txt:
##
@@ -43,7 +43,7 @@ celix_deprecated_utils_headers(test_utils)
configure_file(resources/properties.txt
${CMAKE_CURRENT_BINARY_DIR}/resources-test/properties.txt COPYONLY)
if (CELIX_CXX17)
-add_library(test_utils_cxx17tests STATIC
+add_library(test_utils_cxx17tests OBJECT
Review Comment:
I have not figured out why static library does not work.
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1378446643
##
libs/rcm/src/celix_capability.c:
##
@@ -83,22 +83,19 @@ bool celix_capability_equals(const celix_capability_t*
cap1, const celix_capabil
//compare attributes
bool equals = true;
-const char* visit;
-CELIX_PROPERTIES_FOR_EACH(cap1->attributes, visit) {
-const char* value1 = celix_properties_get(cap1->attributes, visit,
NULL);
-const char* value2 = celix_properties_get(cap2->attributes, visit,
NULL);
-if (!celix_utils_stringEquals(value1, value2)) {
+CELIX_PROPERTIES_ITERATE(cap1->attributes, visit) {
Review Comment:
Shall we use `celix_properties_equals` instead?
##
libs/utils/gtest/src/CxxPropertiesTestSuite.cc:
##
@@ -74,17 +97,111 @@ TEST_F(CxxPropertiesTestSuite, testCopy) {
EXPECT_EQ(v2, "value1_new");
}
-TEST_F(CxxPropertiesTestSuite, testWrap) {
+TEST_F(CxxPropertiesTestSuite, WrapTest) {
auto *props = celix_properties_create();
celix_properties_set(props, "test", "test");
EXPECT_EQ(1, celix_properties_size(props));
{
auto cxxProps = celix::Properties::wrap(props);
-EXPECT_EQ(1, cxxProps->size());
-EXPECT_EQ(props, cxxProps->getCProperties());
+EXPECT_EQ(1, cxxProps.size());
+EXPECT_EQ(props, cxxProps.getCProperties());
+} //NOTE cxxProps out of scope, but will not destroy celix_properties
+EXPECT_EQ(1, celix_properties_size(props));
+
+celix_properties_destroy(props);
+}
+
+TEST_F(CxxPropertiesTestSuite, CopyCPropsTest) {
+auto *props = celix_properties_create();
+celix_properties_set(props, "test", "test");
+
+EXPECT_EQ(1, celix_properties_size(props));
+{
+auto cxxProps = celix::Properties::copy(props);
+EXPECT_EQ(1, cxxProps.size());
+EXPECT_NE(props, cxxProps.getCProperties());
} //NOTE cxxProps out of scope, but will not destroy celix_properties
EXPECT_EQ(1, celix_properties_size(props));
celix_properties_destroy(props);
}
+
+TEST_F(CxxPropertiesTestSuite, GetTypeTest) {
+celix::Properties props{};
+
+const auto v2 = celix::Version{1, 2, 3};
+auto v3 = celix::Version{1, 2, 4};
+
+props.set("bool", true);
+props.set("long1", 1l);
+props.set("long2", (int)1); //should lead to long;
+props.set("long3", (unsigned int)1); //should lead to long;
+props.set("long4", (short)1); //should lead to long;
+props.set("long5", (unsigned short)1); //should lead to long;
+props.set("long6", (char)1); //should lead to long;
+props.set("long7", (unsigned char)1); //should lead to long;
+props.set("double1", 1.0);
+props.set("double2", 1.0f); //set float should lead to double
+props.set("version1", celix::Version{1, 2, 3});
+props.set("version2", v2);
+props.set("version3", v3);
+
+EXPECT_EQ(props.getType("bool"), celix::Properties::ValueType::Bool);
+EXPECT_EQ(props.getType("long1"), celix::Properties::ValueType::Long);
+EXPECT_EQ(props.getType("long2"), celix::Properties::ValueType::Long);
+EXPECT_EQ(props.getType("long3"), celix::Properties::ValueType::Long);
+EXPECT_EQ(props.getType("long4"), celix::Properties::ValueType::Long);
+EXPECT_EQ(props.getType("long5"), celix::Properties::ValueType::Long);
+EXPECT_EQ(props.getType("long6"), celix::Properties::ValueType::Long);
+EXPECT_EQ(props.getType("long7"), celix::Properties::ValueType::Long);
+EXPECT_EQ(props.getType("double1"), celix::Properties::ValueType::Double);
+EXPECT_EQ(props.getType("double2"), celix::Properties::ValueType::Double);
+EXPECT_EQ(props.getType("version1"),
celix::Properties::ValueType::Version);
+EXPECT_EQ(props.getType("version2"),
celix::Properties::ValueType::Version);
+EXPECT_EQ(props.getType("version3"),
celix::Properties::ValueType::Version);
+}
+
+TEST_F(CxxPropertiesTestSuite, GetAsVersionTest) {
+celix::Properties props;
+
+// Test getting a version from a string property
+props.set("key", "1.2.3");
+celix::Version ver{1, 2, 3};
+EXPECT_TRUE(props.getAsVersion("key") == ver);
+
+// Test getting a version from a version property
+props.set("key", celix::Version{2, 3, 4});
+ver = celix::Version{2, 3, 4};
+EXPECT_EQ(props.getAsVersion("key"), ver);
+
+// Test getting default value when property is not set
+ver = celix::Version{3, 4, 5};
+EXPECT_EQ(props.getAsVersion("non_existent_key", celix::Version{3, 4, 5}),
ver);
+
+// Test getting default value when property value is not a valid version
string
+props.set("key", "invalid_version_string");
+ver = celix::Version{4, 5, 6};
+EXPECT_EQ(props.getAsVersion("key", celix::Version{4, 5, 6}), ver);
+}
+
+TEST_F(CxxPropertiesTestSuite, StoreAndLoadTest) {
+std::string path{"cxx_store_and_load_test.properties"};
+
+celix::Properties props{};
+props.set("key1
Re: [PR] type support for properties (celix)
xuzhenbao commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1378321550
##
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:
I think `discoveryZeroconfAnnouncer_copyPropertiesToTxtRecord` also should
be invoked before `while (!celix_propertiesIterator_isEnd(&propIter))` .
otherwise, a same property is set in `txtRecord`.
We can resolve it in another way: Only let
`discoveryZeroconfAnnouncer_copyPropertiesToTxtRecord` do
`celix_propertiesIterator_next` whether txtRecord is filled or not.
~~~
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;
while (!celix_propertiesIterator_isEnd(propIter)) {
key = propIter->key;
val = propIter->entry.value;
if (key) {
DNSServiceErrorType err = TXTRecordSetValue(txtRecord, key,
strlen(val), val);
if (err != kDNSServiceErr_NoError) {
celix_logHelper_error(announcer->logHelper, "Announcer:
Failed to set txt value, %d.", err);
return false;
}
if (splitTxtRecord && TXTRecordGetLength(txtRecord) >= maxTxtLen
- UINT8_MAX) {
celix_propertiesIterator_next(propIter);
break;
}
}
celix_propertiesIterator_next(propIter);
}
return true;
}
~~~
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1378319542
##
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:
> When adding a property with a string value if NULL, a empty string ("")
will be used instead using the celix_properties_createString function
This is nice and improves general user experience.
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1377967375
##
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:
A value cannot be NULL (see previous comment) and a NULL value as key is
also not accepted:
```
static celix_status_t celix_properties_createAndSetEntry(celix_properties_t*
properties,
const char* key,
const char**
strValue,
const long*
longValue,
const double*
doubleValue,
const bool*
boolValue,
celix_version_t*
versionValue) {
if (!properties) {
return CELIX_SUCCESS; // silently ignore
}
if (!key) {
celix_err_push("Cannot set property with NULL key");
return CELIX_SUCCESS; // print error and ignore
}
...
```
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1377967375
##
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:
A value cannot be NULL (see previous comment) and a NULL value as key is
also not accepted:
```
static celix_status_t celix_properties_createAndSetEntry(celix_properties_t*
properties,
const char* key,
const char**
strValue,
const long*
longValue,
const double*
doubleValue,
const bool*
boolValue,
celix_version_t*
versionValue) {
if (!properties) {
return CELIX_SUCCESS; // silently ignore
}
if (!key) {
celix_err_push("Cannot set property with NULL key");
return CELIX_SUCCESS; // print error and ignore
}
```
--
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]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1377963635
##
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:
val cannot be null. When adding a property with a string value if NULL, a
empty string ("") will be used instead using the celix_properties_createString
function:
```
char* celix_properties_createString(celix_properties_t* properties, const
char* str) {
if (str == NULL) {
return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
}
size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
size_t left = CELIX_SHORT_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE -
properties->currentStringBufferIndex;
char* result;
if (len < left) {
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str,
len);
result =
&properties->stringBuffer[properties->currentStringBufferIndex];
properties->currentStringBufferIndex += (int)len;
} else {
result = celix_utils_strdup(str);
}
return result;
}
```
As result val cannot be NULL. I will update the properties iterator
documentation to make this more explicit.
--
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]
Re: [PR] type support for properties (celix)
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] type support for properties (celix)
pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1375760300
##
libs/utils/include/celix/Version.h:
##
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include
+#include
+
+#include "celix_version.h"
+
+namespace celix {
+
+/**
+ * @class Version
+ * @brief Class for storing and manipulating version information.
+ *
+ * The Version class represents a version number that follows the Semantic
Versioning specification (SemVer).
+ * It consists of three non-negative integers for the major, minor, and
micro version, and an optional string for
+ * the qualifier.
+ * The Version class provides comparison operators and functions for
getting the individual version components.
+ *
+ * @note This class is a thin wrapper around the C API defined in
celix_version.h.
+ */
+class Version {
+public:
+
+///@brief Constructs a new empty version with all components set to
zero.
+Version() :
+cVersion{createVersion(celix_version_createEmptyVersion())},
+qualifier{celix_version_getQualifier(cVersion.get())} {}
+
+/**
+ * @brief Constructs a new version with the given components and
qualifier.
+ * @param major The major component of the version.
+ * @param minor The minor component of the version.
+ * @param micro The micro component of the version.
+ * @param qualifier The qualifier string of the version.
+ */
+#if __cplusplus >= 201703L //C++17 or higher
Review Comment:
Remove C++17 string_view usage
--
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]
Re: [PR] type support for properties (celix)
PengZheng commented on PR #470: URL: https://github.com/apache/celix/pull/470#issuecomment-1777000802 I'll try to finish reviewing this PR this week. -- 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]
Re: [PR] type support for properties (celix)
pnoltes commented on PR #470: URL: https://github.com/apache/celix/pull/470#issuecomment-1776990385 This PR is now ready for review. As noted earlier this PR has breaking changes, so the PR was on hold until Apache Celix 2.4.0 was released. Some additional changes added based on the previous review comments: - Updated celix_string/long_hash_map error handling, including a `celix_status_t` return for the `put` functions. Note that before this change the `put` functions returned the previous entry and now returns a status to indicate whether the `put` was successful. - Update properties `set` function to return a `celix_status_t` (this was a `void`). - Added error injection test to check the error handling of hash map put and create, celix properties create, set, store and load. - Added `equals` function for string/long hash map en properties - Removed the deprecated iterator functions and macro. There is still some work todo for storing, encoding any maybe arrays handling for properties but IMO this PR is already big enough so I created a separate issues for this: #674 . -- 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]
