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]

Reply via email to