PengZheng commented on code in PR #692: URL: https://github.com/apache/celix/pull/692#discussion_r1415636865
########## libs/utils/src/filter.c: ########## @@ -17,421 +17,516 @@ * 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 convertedToVersion; - 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); + bool convertedToBool; + bool boolValue; -static celix_status_t filter_compare(const celix_filter_t* filter, const char *propertyValue, bool *result); + bool convertedToVersion; + celix_version_t* versionValue; +}; -static void filter_skipWhiteSpace(char * filterString, int * pos) { - int length; +static void celix_filter_skipWhiteSpace(char* filterString, int* pos); +static celix_filter_t* celix_filter_parseFilter(char* filterString, int* pos); +static celix_filter_t* celix_filter_parseFilterNode(char* filterString, int* pos); +static celix_filter_t* celix_filter_parseAndOrOr(char* filterString, celix_filter_operand_t andOrOr, int* pos); +static celix_filter_t* celix_filter_parseNot(char* filterString, int* pos); +static celix_filter_t* celix_filter_parseItem(char* filterString, int* pos); +static char* celix_filter_parseAttributeOrValue(const char* filterString, int* pos, bool parseAttribute); +static celix_array_list_t* celix_filter_parseSubstring(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(char* filterString, int* pos) { + size_t length; for (length = strlen(filterString); (*pos < length) && 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(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); - 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(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(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(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); Review Comment: Error handling is needed. This remark applies to several places in this file. -- 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