PengZheng commented on code in PR #692:
URL: https://github.com/apache/celix/pull/692#discussion_r1421694792


##########
libs/utils/src/filter.c:
##########
@@ -17,421 +17,497 @@
  * under the License.
  */
 
+#include <assert.h>
+#include <ctype.h>
 #include <stdio.h>
-#include <string.h>
 #include <stdlib.h>
-#include <ctype.h>
-#include <assert.h>
+#include <string.h>
 #include <utils.h>
 
-#include "celix_filter.h"
-#include "filter.h"
+#include "celix_convert_utils.h"
+#include "celix_err.h"
 #include "celix_errno.h"
+#include "celix_filter.h"
+#include "celix_stdio_cleanup.h"
+#include "celix_stdlib_cleanup.h"
 #include "celix_version.h"
-#include "celix_convert_utils.h"
+#include "filter.h"
+
+// ignoring clang-tidy recursion warnings for this file, because filter uses 
recursion
+// NOLINTBEGIN(misc-no-recursion)
 
 struct celix_filter_internal {
     bool convertedToLong;
     long longValue;
+
     bool convertedToDouble;
     double doubleValue;
+
+    bool convertedToBool;
+    bool boolValue;
+
     bool convertedToVersion;
-    celix_version_t *versionValue;
+    celix_version_t* versionValue;
 };
 
-static void filter_skipWhiteSpace(char* filterString, int* pos);
-static celix_filter_t * filter_parseFilter(char* filterString, int* pos);
-static celix_filter_t * filter_parseFilterComp(char* filterString, int* pos);
-static celix_filter_t * filter_parseAndOrOr(char* filterString, 
celix_filter_operand_t andOrOr, int* pos);
-static celix_filter_t * filter_parseNot(char* filterString, int* pos);
-static celix_filter_t * filter_parseItem(char* filterString, int* pos);
-static char * filter_parseAttr(char* filterString, int* pos);
-static char * filter_parseValue(char* filterString, int* pos);
-static celix_array_list_t* filter_parseSubstring(char* filterString, int* pos);
-
-static celix_status_t filter_compare(const celix_filter_t* filter, const char 
*propertyValue, bool *result);
-
-static void filter_skipWhiteSpace(char * filterString, int * pos) {
-    int length;
-    for (length = strlen(filterString); (*pos < length) && 
isspace(filterString[*pos]);) {
+static void celix_filter_skipWhiteSpace(const char* filterString, int* pos);
+static celix_filter_t* celix_filter_parseFilter(const char* filterString, int* 
pos);
+static celix_filter_t* celix_filter_parseFilterNode(const char* filterString, 
int* pos);
+static celix_filter_t* celix_filter_parseAndOrOr(const char* filterString, 
celix_filter_operand_t andOrOr, int* pos);
+static celix_filter_t* celix_filter_parseNot(const char* filterString, int* 
pos);
+static celix_filter_t* celix_filter_parseItem(const char* filterString, int* 
pos);
+static char* celix_filter_parseAttributeOrValue(const char* filterString, int* 
pos, bool parseAttribute);
+static celix_array_list_t* celix_filter_parseSubstring(const char* 
filterString, int* pos);
+static bool celix_filter_isSubString(const char* filterString, int startPos);
+static bool celix_filter_isNextNonWhiteSpaceChar(const char* filterString, int 
pos, char c);
+
+static void celix_filter_skipWhiteSpace(const char* filterString, int* pos) {
+    for (; filterString[*pos] != '\0' && isspace(filterString[*pos]);) {
         (*pos)++;
     }
 }
 
-celix_filter_t * filter_create(const char* filterString) {
-    return celix_filter_create(filterString);
+static bool celix_filter_isNextNonWhiteSpaceChar(const char* filterString, int 
pos, char c) {
+    for (int i = pos; filterString[i] != '\0'; i++) {
+        if (!isspace(filterString[i])) {
+            return filterString[i] == c;
+        }
+    }
+    return false;
 }
 
-void filter_destroy(celix_filter_t * filter) {
-    return celix_filter_destroy(filter);
-}
+celix_filter_t* filter_create(const char* filterString) { return 
celix_filter_create(filterString); }
 
-static celix_filter_t * filter_parseFilter(char* filterString, int* pos) {
-    celix_filter_t * filter;
-    filter_skipWhiteSpace(filterString, pos);
-    if (filterString[*pos] == '\0') {
-        //empty filter
-        fprintf(stderr, "Filter Error: Cannot create LDAP filter from an empty 
string.\n");
-        return NULL;
-    } else if (filterString[*pos] != '(') {
-        fprintf(stderr, "Filter Error: Missing '(' in filter string '%s'.\n", 
filterString);
+void filter_destroy(celix_filter_t* filter) { return 
celix_filter_destroy(filter); }
+
+static celix_filter_t* celix_filter_parseFilter(const char* filterString, int* 
pos) {
+    celix_filter_skipWhiteSpace(filterString, pos);
+    if (filterString[*pos] != '(') {
+        celix_err_pushf("Filter Error: Missing '(' in filter string '%s'.", 
filterString);
         return NULL;
     }
-    (*pos)++; //eat '('
+    (*pos)++; // eat '('
 
-    filter = filter_parseFilterComp(filterString, pos);
+    celix_autoptr(celix_filter_t) filter = 
celix_filter_parseFilterNode(filterString, pos);
+    if (!filter) {
+        return NULL;
+    }
 
-    filter_skipWhiteSpace(filterString, pos);
+    celix_filter_skipWhiteSpace(filterString, pos);
 
     if (filterString[*pos] != ')') {
-        fprintf(stderr, "Filter Error: Missing ')' in filter string '%s'.\n", 
filterString);
-        if(filter!=NULL){
-            filter_destroy(filter);
-        }
+        celix_err_pushf("Filter Error: Missing ')' in filter string '%s'.", 
filterString);
         return NULL;
     }
-    (*pos)++; //eat ')'
-    filter_skipWhiteSpace(filterString, pos);
+    (*pos)++; // eat ')'
+    celix_filter_skipWhiteSpace(filterString, pos);
 
-    return filter;
+    return celix_steal_ptr(filter);
 }
 
-static celix_filter_t * filter_parseFilterComp(char * filterString, int * pos) 
{
+static celix_filter_t* celix_filter_parseFilterNode(const char* filterString, 
int* pos) {
     char c;
-    filter_skipWhiteSpace(filterString, pos);
+    celix_filter_skipWhiteSpace(filterString, pos);
 
     c = filterString[*pos];
 
     switch (c) {
-        case '&': {
-            (*pos)++;
-            return filter_parseAndOrOr(filterString, CELIX_FILTER_OPERAND_AND, 
pos);
-        }
-        case '|': {
-            (*pos)++;
-            return filter_parseAndOrOr(filterString, CELIX_FILTER_OPERAND_OR, 
pos);
-        }
-        case '!': {
-            (*pos)++;
-            return filter_parseNot(filterString, pos);
-        }
+    case '&': {
+        (*pos)++;
+        return celix_filter_parseAndOrOr(filterString, 
CELIX_FILTER_OPERAND_AND, pos);
+    }
+    case '|': {
+        (*pos)++;
+        return celix_filter_parseAndOrOr(filterString, 
CELIX_FILTER_OPERAND_OR, pos);
+    }
+    case '!': {
+        (*pos)++;
+        return celix_filter_parseNot(filterString, pos);
+    }
+    default: {
+        return celix_filter_parseItem(filterString, pos);
+    }
     }
-    return filter_parseItem(filterString, pos);
 }
 
-static celix_filter_t * filter_parseAndOrOr(char * filterString, 
celix_filter_operand_t andOrOr, int * pos) {
+static celix_filter_t* celix_filter_parseAndOrOr(const char* filterString, 
celix_filter_operand_t andOrOr, int* pos) {
+    celix_filter_skipWhiteSpace(filterString, pos);
 
-    filter_skipWhiteSpace(filterString, pos);
-    bool failure = false;
-
-    if (filterString[*pos] != '(') {
-        fprintf(stderr, "Filter Error: Missing '('.\n");
+    celix_autoptr(celix_filter_t) filter = (celix_filter_t*)calloc(1, 
sizeof(*filter));
+    if (!filter) {
+        celix_err_push("Filter Error: Failed to allocate memory.");
         return NULL;
     }
 
-    celix_array_list_t *children = celix_arrayList_create();
-    while(filterString[*pos] == '(') {
-        celix_filter_t * child = filter_parseFilter(filterString, pos);
-        if(child == NULL) {
-            failure = true;
-            break;
-        }
-        celix_arrayList_add(children, child);
+    celix_array_list_create_options_t ops = 
CELIX_EMPTY_ARRAY_LIST_CREATE_OPTIONS;
+    ops.simpleRemovedCallback = (void (*)(void*))celix_filter_destroy;
+    celix_autoptr(celix_array_list_t) children = 
celix_arrayList_createWithOptions(&ops);
+    if (!children) {
+        celix_err_push("Filter Error: Failed to allocate memory.");
+        return NULL;
     }
 
-    if(failure == true){
-        for (int i = 0; i < celix_arrayList_size(children); ++i) {
-            celix_filter_t * f = celix_arrayList_get(children, i);
-            filter_destroy(f);
-        }
-        celix_arrayList_destroy(children);
+    if (filterString[*pos] == ')') {
+        // empty and/or filter
+    } else if (filterString[*pos] != '(') {
+        celix_err_push("Filter Error: Missing '('.");
         return NULL;
+    } else {
+        while (filterString[*pos] == '(') {
+            celix_filter_t* child = celix_filter_parseFilter(filterString, 
pos);
+            if (child == NULL) {
+                return NULL;
+            }
+            if(celix_arrayList_add(children, child) != CELIX_SUCCESS) {
+                celix_err_push("Filter Error: Failed to add filter node.");
+                celix_filter_destroy(child);
+                return NULL;
+            }
+        }
     }
 
-    celix_filter_t * filter = (celix_filter_t *) calloc(1, sizeof(*filter));
     filter->operand = andOrOr;
-    filter->children = children;
+    filter->children = celix_steal_ptr(children);
 
-    return filter;
+    return celix_steal_ptr(filter);
 }
 
-static celix_filter_t * filter_parseNot(char * filterString, int * pos) {
-    celix_filter_t * child = NULL;
-    filter_skipWhiteSpace(filterString, pos);
+static celix_filter_t* celix_filter_parseNot(const char* filterString, int* 
pos) {
+    celix_filter_skipWhiteSpace(filterString, pos);
 
-    if (filterString[*pos] != '(') {
-        fprintf(stderr, "Filter Error: Missing '('.\n");
+    char c = filterString[*pos];
+    if (c != '(') {
+        celix_err_push("Filter Error: Missing '('.");
         return NULL;
     }
 
-    child = filter_parseFilter(filterString, pos);
-    if (child == NULL) {
+    celix_autoptr(celix_filter_t) child = 
celix_filter_parseFilter(filterString, pos);
+    if (!child) {
         return NULL;
     }
-    celix_array_list_t* children = celix_arrayList_create();
-    celix_arrayList_add(children, child);
 
-    celix_filter_t * filter = (celix_filter_t *) calloc(1, sizeof(*filter));
-    filter->operand = CELIX_FILTER_OPERAND_NOT;
-    filter->children = children;
+    celix_array_list_create_options_t ops = 
CELIX_EMPTY_ARRAY_LIST_CREATE_OPTIONS;
+    ops.simpleRemovedCallback = (void (*)(void*))celix_filter_destroy;
+    celix_autoptr(celix_array_list_t) children = 
celix_arrayList_createWithOptions(&ops);
+    if (!children) {
+        celix_err_push("Filter Error: Failed to allocate memory.");
+        return NULL;
+    }
+    if (celix_arrayList_add(children, child) != CELIX_SUCCESS) {
+        celix_err_push("Filter Error: Failed to add NOT filter node.");
+        return NULL;
+    }
+    child = NULL;
 
+    celix_filter_t* filter = (celix_filter_t*)calloc(1, sizeof(*filter));
+    if (!filter) {
+        celix_err_push("Filter Error: Failed to allocate memory.");
+        return NULL;
+    }
+
+    filter->operand = CELIX_FILTER_OPERAND_NOT;
+    filter->children = celix_steal_ptr(children);
     return filter;
 }
 
-static celix_filter_t * filter_parseItem(char * filterString, int * pos) {
-    char * attr = filter_parseAttr(filterString, pos);
-    if(attr == NULL){
+static celix_filter_t* celix_filter_parseItem(const char* filterString, int* 
pos) {
+    celix_autofree char* attr = 
celix_filter_parseAttributeOrValue(filterString, pos, true);
+    if (!attr) {
+        celix_err_push("Filter Error: Missing attr.");
         return NULL;
     }
 
-    filter_skipWhiteSpace(filterString, pos);
-    switch(filterString[*pos]) {
-        case '~': {
-            if (filterString[*pos + 1] == '=') {
-                celix_filter_t * filter = (celix_filter_t *) calloc(1, 
sizeof(*filter));
-                *pos += 2;
-                filter->operand = CELIX_FILTER_OPERAND_APPROX;
-                filter->attribute = attr;
-                filter->value = filter_parseValue(filterString, pos);
-                return filter;
+    celix_autoptr(celix_filter_t) filter = calloc(1, sizeof(*filter));
+    if (!filter) {
+        celix_err_push("Filter Error: Failed to allocate memory.");
+        return NULL;
+    }
+    filter->attribute = celix_steal_ptr(attr);
+
+    celix_filter_skipWhiteSpace(filterString, pos);
+    char op = filterString[*pos];
+    switch (op) {
+    case '~': {
+        char secondOp = filterString[*pos + 1];
+        if (secondOp == '=') {
+            *pos += 2;
+            filter->operand = CELIX_FILTER_OPERAND_APPROX;
+            filter->value = celix_filter_parseAttributeOrValue(filterString, 
pos, false);
+            if (!filter->value) {
+                return NULL;
             }
             break;
         }
-        case '>': {
-            if (filterString[*pos + 1] == '=') {
-                celix_filter_t * filter = (celix_filter_t *) calloc(1, 
sizeof(*filter));
-                *pos += 2;
-                filter->operand = CELIX_FILTER_OPERAND_GREATEREQUAL;
-                filter->attribute = attr;
-                filter->value = filter_parseValue(filterString, pos);
-                return filter;
-            }
-            else {
-                celix_filter_t * filter = (celix_filter_t *) calloc(1, 
sizeof(*filter));
-                *pos += 1;
-                filter->operand = CELIX_FILTER_OPERAND_GREATER;
-                filter->attribute = attr;
-                filter->value = filter_parseValue(filterString, pos);
-                return filter;
-            }
-            break;
+        celix_err_pushf("Filter Error: Invalid operand char after ~. Expected 
`=` got `%c`", secondOp);
+        return NULL;
+    }
+    case '>': {
+        if (filterString[*pos + 1] == '=') {
+            *pos += 2;
+            filter->operand = CELIX_FILTER_OPERAND_GREATEREQUAL;
+        } else {
+            *pos += 1;
+            filter->operand = CELIX_FILTER_OPERAND_GREATER;
         }
-        case '<': {
-            if (filterString[*pos + 1] == '=') {
-                celix_filter_t * filter = (celix_filter_t *) calloc(1, 
sizeof(*filter));
-                *pos += 2;
-                filter->operand = CELIX_FILTER_OPERAND_LESSEQUAL;
-                filter->attribute = attr;
-                filter->value = filter_parseValue(filterString, pos);
-                return filter;
-            }
-            else {
-                celix_filter_t * filter = (celix_filter_t *) calloc(1, 
sizeof(*filter));
-                *pos += 1;
-                filter->operand = CELIX_FILTER_OPERAND_LESS;
-                filter->attribute = attr;
-                filter->value = filter_parseValue(filterString, pos);
-                return filter;
-            }
-            break;
+        filter->value = celix_filter_parseAttributeOrValue(filterString, pos, 
false);
+        if (!filter->value) {
+            return NULL;
         }
-        case '=': {
-            celix_filter_t * filter = NULL;
-            celix_array_list_t *subs;
-            if (filterString[*pos + 1] == '*') {
-                int oldPos = *pos;
-                *pos += 2;
-                filter_skipWhiteSpace(filterString, pos);
-                if (filterString[*pos] == ')') {
-                    filter = (celix_filter_t *) calloc(1, sizeof(*filter));
-                    filter->operand = CELIX_FILTER_OPERAND_PRESENT;
-                    filter->attribute = attr;
-                    filter->value = NULL;
-                    return filter;
-                }
-                *pos = oldPos;
+        break;
+    }
+    case '<': {
+        if (filterString[*pos + 1] == '=') {
+            *pos += 2;
+            filter->operand = CELIX_FILTER_OPERAND_LESSEQUAL;
+        } else {
+            *pos += 1;
+            filter->operand = CELIX_FILTER_OPERAND_LESS;
+        }
+        filter->value = celix_filter_parseAttributeOrValue(filterString, pos, 
false);
+        if (!filter->value) {
+            return NULL;
+        }
+        break;
+    }
+    case '=': {
+        if (filterString[*pos + 1] == '*') {
+            int oldPos = *pos;
+            *pos += 2;
+            celix_filter_skipWhiteSpace(filterString, pos);
+            if (filterString[*pos] == ')') {
+                filter->operand = CELIX_FILTER_OPERAND_PRESENT;
+                filter->value = NULL;
+                break;
             }
-            filter = (celix_filter_t *) calloc(1, sizeof(*filter));            
-            (*pos)++;
-            subs = filter_parseSubstring(filterString, pos);
-            if(subs!=NULL){
-                if (celix_arrayList_size(subs) == 1) {
-                    char * string = (char *) celix_arrayList_get(subs, 0);
-                    if (string != NULL) {
-                        filter->operand = CELIX_FILTER_OPERAND_EQUAL;
-                        filter->attribute = attr;
-                        filter->value = string;
-
-                        celix_arrayList_clear(subs);
-                        celix_arrayList_destroy(subs);
-
-                        return filter;
-                    }
-                }
+            *pos = oldPos;
+        }
+        (*pos)++;
+        if (celix_filter_isSubString(filterString, *pos)) {
+            celix_array_list_t* subs = 
celix_filter_parseSubstring(filterString, pos);
+            if (!subs) {
+                return NULL;
             }
             filter->operand = CELIX_FILTER_OPERAND_SUBSTRING;
-            filter->attribute = attr;
             filter->children = subs;
-            return filter;
-        }
-    }
-    fprintf(stderr, "Filter Error: Invalid operator.\n");
-    free(attr);
-    return NULL;
-}
-
-static char * filter_parseAttr(char * filterString, int * pos) {
-    char c;
-    int begin = *pos;
-    int end = *pos;
-    int length = 0;
-
-    filter_skipWhiteSpace(filterString, pos);
-    c = filterString[*pos];
-
-    while (c != '~' && c != '<' && c != '>' && c != '=' && c != '(' && c != 
')') {
-        (*pos)++;
-
-        if (!isspace(c)) {
-            end = *pos;
+        } else {
+            filter->operand = CELIX_FILTER_OPERAND_EQUAL;
+            filter->value = celix_filter_parseAttributeOrValue(filterString, 
pos, false);
+            if (!filter->value) {
+                return NULL;
+            }
         }
-
-        c = filterString[*pos];
+        break;
     }
-
-    length = end - begin;
-
-    if (length == 0) {
-        fprintf(stderr, "Filter Error: Missing attr.\n");
+    default: {
+        celix_err_pushf("Filter Error: Invalid operand char `%c`", op);
         return NULL;
-    } else {
-        char * attr = (char *) calloc(1, length+1);
-        strncpy(attr, filterString+begin, length);
-        attr[length] = '\0';
-        return attr;
     }
+    }
+    return celix_steal_ptr(filter);
 }
 
-static char * filter_parseValue(char * filterString, int * pos) {
-    char *value = calloc(strlen(filterString) + 1, sizeof(*value));
-    int keepRunning = 1;
+static char* celix_filter_parseAttributeOrValue(const char* filterString, int* 
pos, bool parseAttribute) {
+    const char* name = parseAttribute ? "attribute" : "attribute value";
+    celix_autofree char* value = NULL;
+    size_t valueSize = 0;
+    celix_autoptr(FILE) stream = NULL;
 
+    bool keepRunning = true;
     while (keepRunning) {
         char c = filterString[*pos];
 
         switch (c) {
-            case ')': {
-                keepRunning = 0;
-                break;
-            }
-            case '(': {
-                fprintf(stderr, "Filter Error: Invalid value.\n");
-                free(value);
+        case ')': {
+            if (parseAttribute) {
+                celix_err_pushf("Filter Error: Unexpected `)` char while 
parsing %s.", name);
                 return NULL;
             }
-            case '\0':{
-                fprintf(stderr, "Filter Error: Unclosed bracket.\n");
-                free(value);
-                return NULL;
+            keepRunning = false;
+            break;
+        }
+        case '(': {
+            celix_err_pushf("Filter Error: Unexpected `(` char while parsing 
%s.", name);
+            return NULL;
+        }
+        case '\0': {
+            celix_err_pushf("Filter Error: Unexpected end of string while 
parsing %s.", name);
+            return NULL;
+        }
+        case '<':
+        case '>':
+        case '=':
+        case '~': {
+            if (parseAttribute) {
+                keepRunning = false; // done for attribute, valid for value
+                break;
             }
-            case '\\': {
-                (*pos)++;
+            /* no break */
+        }
+        default: {
+            if (c == '\\') {
+                if (filterString[*pos + 1] == '\0') {
+                    celix_err_pushf("Filter Error: Unexpected end of string 
while parsing %s.", name);
+                    return NULL;
+                }
+                (*pos)++; // eat '\'
                 c = filterString[*pos];
             }
-            /* no break */
-            default: {
-                char ch[2];
-                ch[0] = c;
-                ch[1] = '\0';
-                strcat(value, ch);
-                (*pos)++;
-                break;
+            if (!stream) {
+                stream = open_memstream(&value, &valueSize);
+                if (!stream) {
+                    celix_err_push("Filter Error: Failed to open mem stream.");
+                    return NULL;
+                }
+            }
+            int rc = fputc(c, stream);
+            if (rc == EOF) {
+                celix_err_push("Filter Error: Failed to write to stream.");
+                return NULL;
             }
+            (*pos)++;
+            break;
+        }
         }
     }
 
-    if (strlen(value) == 0) {
-        fprintf(stderr, "Filter Error: Missing value.\n");
-        free(value);
+    if (!stream) {
+        celix_err_push("Filter Error: Empty value.\n");

Review Comment:
   According to 
[java.util.Properties.load(java.io.Reader)](https://docs.oracle.com/en/java/javase/19/docs/api/java.base/java/util/Properties.html#load(java.io.Reader))
 
   
   >  As a third example, the line:
   cheeses
   specifies that the key is "cheeses" and the associated element is the empty 
string "". 
   
   If empty value is not allowed, we cannot match key with empty value using 
equal filters. 



-- 
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.apache.org

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

Reply via email to