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


##########
libs/utils/src/filter.c:
##########
@@ -17,421 +17,503 @@
  * 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;
+    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;
+    }
 
-    if (filterString[*pos] != '(') {
-        fprintf(stderr, "Filter Error: Missing '('.\n");
+    celix_autoptr(celix_array_list_t) children = celix_arrayList_create();
+    if (!children) {
+        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;
+    if (filterString[*pos] == ')') {
+        // empty and/or filter
+    } else if (filterString[*pos] != '(') {
+        celix_err_push("Filter Error: Missing '('.");
+        return NULL;
+    } else {
+        bool failure = false;
+        while (filterString[*pos] == '(') {
+            celix_filter_t* child = celix_filter_parseFilter(filterString, 
pos);
+            if (child == NULL) {
+                failure = true;
+                break;
+            }
+            celix_arrayList_add(children, child);
         }
-        celix_arrayList_add(children, child);
-    }
 
-    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);
+        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);
+            }
+            return NULL;
         }
-        celix_arrayList_destroy(children);
-        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_filter_t* child = celix_filter_parseFilter(filterString, pos);
+    if (!child) {
         return NULL;
     }
+
     celix_array_list_t* children = celix_arrayList_create();
+    if (!children) {
+        celix_err_push("Filter Error: Failed to allocate memory.");
+        celix_filter_destroy(child);
+        return NULL;
+    }
     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_filter_t* filter = (celix_filter_t*)calloc(1, sizeof(*filter));
+    if (!filter) {
+        celix_err_push("Filter Error: Failed to allocate memory.");
+        celix_filter_destroy(child);
+        celix_arrayList_destroy(children);
+        return NULL;
+    }
 
+    filter->operand = CELIX_FILTER_OPERAND_NOT;
+    filter->children = children; //note can be NULL
     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 = open_memstream(&value, &valueSize);
+    if (!stream) {
+        celix_err_push("Filter Error: Failed to open mem stream.");
+        return 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;
+            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);
+    // end with \0
+    int rc = fputc('\0', stream);

Review Comment:
   According to `man open_memstream`:
   
   > A null byte is maintained at the end of the buffer.  This byte is not 
included in the size value stored at sizeloc.



-- 
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