This is an automated email from the ASF dual-hosted git repository.

pnoltes pushed a commit to branch feature/674-use-properties-type-in-filter
in repository https://gitbox.apache.org/repos/asf/celix.git

commit 535dc4e89de78e3248a21c9e5e90576d82a26afb
Author: Pepijn Noltes <[email protected]>
AuthorDate: Sun Nov 19 23:28:53 2023 +0100

    Refactor filter match to use property typed values
---
 libs/framework/src/bundle_context.c               |  46 ++++---
 libs/framework/src/dm_component_impl.c            |  27 ++--
 libs/framework/src/service_registration.c         |  98 +++++++++------
 libs/framework/src/service_registration_private.h |  16 +--
 libs/utils/gtest/src/FilterTestSuite.cc           |  27 ++++
 libs/utils/src/filter.c                           | 145 ++++++++++++++--------
 6 files changed, 231 insertions(+), 128 deletions(-)

diff --git a/libs/framework/src/bundle_context.c 
b/libs/framework/src/bundle_context.c
index d8b2c95c..f45d8222 100644
--- a/libs/framework/src/bundle_context.c
+++ b/libs/framework/src/bundle_context.c
@@ -43,7 +43,7 @@
 #include "celix_array_list.h"
 #include "celix_convert_utils.h"
 
-static celix_status_t bundleContext_bundleChanged(void *handle, bundle_event_t 
*event);
+static celix_status_t bundleContext_bundleChanged(void* listenerSvc, 
bundle_event_t* event);
 static void bundleContext_cleanupBundleTrackers(bundle_context_t *ct);
 static void bundleContext_cleanupServiceTrackers(bundle_context_t *ctx);
 static void bundleContext_cleanupServiceTrackerTrackers(bundle_context_t *ctx);
@@ -147,7 +147,10 @@ celix_status_t 
bundleContext_installBundle(bundle_context_pt context, const char
     return bundleContext_installBundle2(context, location, NULL, bundle);
 }
 
-celix_status_t bundleContext_installBundle2(bundle_context_pt context, const 
char *location, const char *inputFile, bundle_pt *bundle) {
+celix_status_t bundleContext_installBundle2(bundle_context_pt context,
+                                            const char* location,
+                                            const char* inputFile 
__attribute__((unused)),
+                                            bundle_pt* bundle) {
     celix_status_t status = CELIX_SUCCESS;
     long id = -1L;
     if (context == NULL || location == NULL || bundle == NULL) {
@@ -389,15 +392,23 @@ static long 
celix_bundleContext_registerServiceWithOptionsInternal(bundle_contex
     }
 
     //set properties
-    celix_properties_t *props = opts->properties;
+    celix_autoptr(celix_properties_t) props = opts->properties;
     if (props == NULL) {
         props = celix_properties_create();
     }
+
     if (opts->serviceVersion != NULL && strncmp("", opts->serviceVersion, 1) 
!= 0) {
-        celix_properties_set(props, CELIX_FRAMEWORK_SERVICE_VERSION, 
opts->serviceVersion);
+        celix_version_t* version = 
celix_version_createVersionFromString(opts->serviceVersion);
+        if (!version) {
+            celix_framework_logTssErrors(ctx->framework->logger, 
CELIX_LOG_LEVEL_ERROR);
+            fw_log(
+                ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot parse 
service version %s", opts->serviceVersion);
+            return -1;
+        }
+        celix_properties_setVersionWithoutCopy(props, 
CELIX_FRAMEWORK_SERVICE_VERSION, version);
     }
 
-    long svcId = -1;
+    long svcId;
     if (!async && celix_framework_isCurrentThreadTheEventLoop(ctx->framework)) 
{
         /*
          * Note already on event loop, cannot register the service async, 
because we cannot wait a future event (the
@@ -407,20 +418,17 @@ static long 
celix_bundleContext_registerServiceWithOptionsInternal(bundle_contex
          * registrations versions on the event loop thread
          */
 
-        svcId = celix_framework_registerService(ctx->framework, ctx->bundle, 
opts->serviceName, opts->svc, opts->factory, props);
+        svcId = celix_framework_registerService(ctx->framework, ctx->bundle, 
opts->serviceName, opts->svc, opts->factory, celix_steal_ptr(props));
     } else {
         void (*asyncCallback)(void *data, long serviceId) = async ? 
opts->asyncCallback : NULL; //NOTE for not async call do not use the callback.
-        svcId = celix_framework_registerServiceAsync(ctx->framework, 
ctx->bundle, opts->serviceName, opts->svc, opts->factory, props, 
opts->asyncData, asyncCallback, NULL, NULL);
+        svcId = celix_framework_registerServiceAsync(ctx->framework, 
ctx->bundle, opts->serviceName, opts->svc, opts->factory, 
celix_steal_ptr(props), opts->asyncData, asyncCallback, NULL, NULL);
         if (!async && svcId >= 0) {
             //note on event loop thread, but in a sync call, so waiting till 
service registration is concluded
             celix_bundleContext_waitForAsyncRegistration(ctx, svcId);
         }
     }
 
-
-    if (svcId < 0) {
-        properties_destroy(props);
-    } else {
+    if (svcId >= 0) {
         celixThreadMutex_lock(&ctx->mutex);
         celix_arrayList_addLong(ctx->svcRegistrations, svcId);
         celixThreadMutex_unlock(&ctx->mutex);
@@ -519,19 +527,18 @@ celix_dependency_manager_t* 
celix_bundleContext_getDependencyManager(bundle_cont
     return result;
 }
 
-static celix_status_t bundleContext_bundleChanged(void *listenerSvc, 
bundle_event_t *event) {
-    celix_status_t status = CELIX_SUCCESS;
-    bundle_listener_t *listener = listenerSvc;
-    celix_bundle_context_bundle_tracker_entry_t *tracker = listener->handle;
+static celix_status_t bundleContext_bundleChanged(void* listenerSvc, 
bundle_event_t* event) {
+    bundle_listener_t* listener = listenerSvc;
+    celix_bundle_context_bundle_tracker_entry_t* tracker = listener->handle;
 
     bool handleEvent = true;
     long bndId = celix_bundle_getId(event->bnd);
-    if (bndId == 0 /*framework bundle*/)  {
+    if (bndId == 0 /*framework bundle*/) {
         handleEvent = tracker->opts.includeFrameworkBundle;
     }
 
     if (handleEvent) {
-        void *callbackHandle = tracker->opts.callbackHandle;
+        void* callbackHandle = tracker->opts.callbackHandle;
 
         if (event->type == OSGI_FRAMEWORK_BUNDLE_EVENT_INSTALLED && 
tracker->opts.onInstalled != NULL) {
             tracker->opts.onInstalled(callbackHandle, event->bnd);
@@ -545,7 +552,7 @@ static celix_status_t bundleContext_bundleChanged(void 
*listenerSvc, bundle_even
             tracker->opts.onBundleEvent(callbackHandle, event);
         }
     }
-    return status;
+    return CELIX_SUCCESS;
 }
 
 void celix_bundleContext_trackBundlesWithOptionsCallback(void *data) {
@@ -571,7 +578,6 @@ static long 
celix_bundleContext_trackBundlesWithOptionsInternal(
         bundle_context_t* ctx,
         const celix_bundle_tracking_options_t *opts,
         bool async) {
-    long trackerId = -1;
     celix_bundle_context_bundle_tracker_entry_t *entry = calloc(1, 
sizeof(*entry));
     memcpy(&entry->opts, opts, sizeof(*opts));
     entry->ctx = ctx;
@@ -582,7 +588,7 @@ static long 
celix_bundleContext_trackBundlesWithOptionsInternal(
     celixThreadMutex_lock(&ctx->mutex);
     entry->trackerId = ctx->nextTrackerId++;
     hashMap_put(ctx->bundleTrackers, (void*)(entry->trackerId), entry);
-    trackerId = entry->trackerId;
+    long trackerId = entry->trackerId;
     celixThreadMutex_unlock(&ctx->mutex);
 
     if (!async) { //note only using the async callback if this is a async call.
diff --git a/libs/framework/src/dm_component_impl.c 
b/libs/framework/src/dm_component_impl.c
index 8ba8def3..1286a1ef 100644
--- a/libs/framework/src/dm_component_impl.c
+++ b/libs/framework/src/dm_component_impl.c
@@ -445,29 +445,42 @@ celix_status_t 
component_addInterface(celix_dm_component_t *component, const cha
     return celix_dmComponent_addInterface(component, serviceName, 
serviceVersion, service, properties);
 }
 
-celix_status_t celix_dmComponent_addInterface(celix_dm_component_t *component, 
const char* serviceName, const char* serviceVersion, const void* service, 
celix_properties_t* properties) {
+celix_status_t celix_dmComponent_addInterface(celix_dm_component_t* component,
+                                              const char* serviceName,
+                                              const char* serviceVersion,
+                                              const void* service,
+                                              celix_properties_t* properties) {
     if (serviceName == NULL || celix_utils_stringEquals(serviceName, "")) {
-        celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_ERROR, 
"Cannot add interface with a NULL or empty serviceName");
+        celix_bundleContext_log(
+            component->context, CELIX_LOG_LEVEL_ERROR, "Cannot add interface 
with a NULL or empty serviceName");
         return CELIX_ILLEGAL_ARGUMENT;
     }
 
-    dm_interface_t *interface = calloc(1, sizeof(*interface));
-    char *name = celix_utils_strdup(serviceName);
+    dm_interface_t* interface = calloc(1, sizeof(*interface));
+    char* name = celix_utils_strdup(serviceName);
 
     if (properties == NULL) {
         properties = celix_properties_create();
     }
 
-    if ((properties_get(properties, CELIX_FRAMEWORK_SERVICE_VERSION) == NULL) 
&& (serviceVersion != NULL)) {
-        celix_properties_set(properties, CELIX_FRAMEWORK_SERVICE_VERSION, 
serviceVersion);
+    if (serviceVersion != NULL) {
+        celix_version_t* version = 
celix_version_createVersionFromString(serviceVersion);
+        if (!version) {
+            celix_bundleContext_log(
+                component->context, CELIX_LOG_LEVEL_ERROR, "Cannot add 
interface with an invalid serviceVersion");
+            celix_properties_destroy(properties);
+            return CELIX_ILLEGAL_ARGUMENT;
+        }
+        celix_properties_setVersionWithoutCopy(properties, 
CELIX_FRAMEWORK_SERVICE_VERSION, version);
     }
+
     celix_properties_set(properties, CELIX_DM_COMPONENT_UUID, 
(char*)component->uuid);
 
     celixThreadMutex_lock(&component->mutex);
     interface->serviceName = name;
     interface->service = service;
     interface->properties = properties;
-    interface->svcId= -1L;
+    interface->svcId = -1L;
     celix_arrayList_add(component->providedInterfaces, interface);
     if (celix_dmComponent_currentState(component) == 
CELIX_DM_CMP_STATE_TRACKING_OPTIONAL) {
         celix_dmComponent_registerServices(component, false);
diff --git a/libs/framework/src/service_registration.c 
b/libs/framework/src/service_registration.c
index 5564d331..45b89c6e 100644
--- a/libs/framework/src/service_registration.c
+++ b/libs/framework/src/service_registration.c
@@ -21,52 +21,66 @@
 #include <stdio.h>
 #include <string.h>
 
-#include "service_registration_private.h"
-#include "celix_constants.h"
 #include "celix_build_assert.h"
+#include "celix_constants.h"
+#include "celix_err.h"
+#include "service_registration_private.h"
 
 static bool serviceRegistration_destroy(struct celix_ref *);
-static celix_status_t 
serviceRegistration_initializeProperties(service_registration_pt registration, 
properties_pt properties);
-static celix_status_t serviceRegistration_createInternal(registry_callback_t 
callback, bundle_pt bundle, const char* serviceName, unsigned long serviceId,
-        const void * serviceObject, properties_pt dictionary, enum 
celix_service_type svcType, service_registration_pt *registration);
-
-service_registration_pt serviceRegistration_create(registry_callback_t 
callback, bundle_pt bundle, const char* serviceName, unsigned long serviceId, 
const void * serviceObject, properties_pt dictionary) {
+static celix_status_t 
serviceRegistration_initializeProperties(service_registration_t*registration,
+                                                               
celix_properties_t* props);
+
+static celix_status_t serviceRegistration_createInternal(registry_callback_t 
callback,
+                                                         celix_bundle_t* 
bundle,
+                                                         const char* 
serviceName,
+                                                         long serviceId,
+                                                         const void* 
serviceObject,
+                                                         celix_properties_t* 
dictionary,
+                                                         enum 
celix_service_type svcType,
+                                                         
service_registration_t** out);
+
+service_registration_pt serviceRegistration_create(registry_callback_t 
callback, bundle_pt bundle, const char* serviceName, long serviceId, const void 
* serviceObject, properties_pt dictionary) {
     service_registration_pt registration = NULL;
        serviceRegistration_createInternal(callback, bundle, serviceName, 
serviceId, serviceObject, dictionary, CELIX_PLAIN_SERVICE, &registration);
        return registration;
 }
 
-service_registration_pt 
serviceRegistration_createServiceFactory(registry_callback_t callback, 
bundle_pt bundle, const char* serviceName, unsigned long serviceId, const void 
* serviceObject, properties_pt dictionary) {
+service_registration_pt 
serviceRegistration_createServiceFactory(registry_callback_t callback, 
bundle_pt bundle, const char* serviceName, long serviceId, const void * 
serviceObject, properties_pt dictionary) {
     service_registration_pt registration = NULL;
     serviceRegistration_createInternal(callback, bundle, serviceName, 
serviceId, serviceObject, dictionary, CELIX_DEPRECATED_FACTORY_SERVICE, 
&registration);
     return registration;
 }
 
-static celix_status_t serviceRegistration_createInternal(registry_callback_t 
callback, bundle_pt bundle, const char* serviceName, unsigned long serviceId,
-                                                         const void * 
serviceObject, properties_pt dictionary, enum celix_service_type svcType, 
service_registration_pt *out) {
-
+static celix_status_t serviceRegistration_createInternal(registry_callback_t 
callback,
+                                                         celix_bundle_t* 
bundle,
+                                                         const char* 
serviceName,
+                                                         long serviceId,
+                                                         const void* 
serviceObject,
+                                                         celix_properties_t* 
dictionary,
+                                                         enum 
celix_service_type svcType,
+                                                         
service_registration_t** out) {
     celix_status_t status = CELIX_SUCCESS;
-       service_registration_pt  reg = calloc(1, sizeof(*reg));
+    service_registration_pt reg = calloc(1, sizeof(*reg));
     if (reg) {
         celix_ref_init(&reg->refCount);
         reg->callback = callback;
-               reg->svcType = svcType;
-               reg->className = strndup(serviceName, 1024*10);
+        reg->svcType = svcType;
+        reg->className = strndup(serviceName, 1024 * 10);
         reg->bundle = bundle;
-               reg->serviceId = serviceId;
-           reg->svcObj = serviceObject;
-               reg->isUnregistering = false;
-               celixThreadRwlock_create(&reg->lock, NULL);
-               serviceRegistration_initializeProperties(reg, dictionary);
-       } else {
-               status = CELIX_ENOMEM;
-       }
+        reg->serviceId = serviceId;
+        reg->svcObj = serviceObject;
+        reg->isUnregistering = false;
+        celixThreadRwlock_create(&reg->lock, NULL);
+        serviceRegistration_initializeProperties(reg, dictionary);
+    } else {
+        status = CELIX_ENOMEM;
+    }
 
-       if (status == CELIX_SUCCESS) {
-               *out = reg;
-       }
+    if (status == CELIX_SUCCESS) {
+        *out = reg;
+    }
 
-       return status;
+    return status;
 }
 
 void serviceRegistration_retain(service_registration_pt registration) {
@@ -91,24 +105,26 @@ static bool serviceRegistration_destroy(struct celix_ref 
*refCount) {
     return true;
 }
 
-static celix_status_t 
serviceRegistration_initializeProperties(service_registration_pt registration, 
properties_pt dictionary) {
-    char sId[32];
-
-       if (dictionary == NULL) {
-               dictionary = properties_create();
-       }
-
-
-       snprintf(sId, 32, "%lu", registration->serviceId);
-       properties_set(dictionary, (char *) CELIX_FRAMEWORK_SERVICE_ID, sId);
+static celix_status_t 
serviceRegistration_initializeProperties(service_registration_t*registration,
+                                                               
celix_properties_t* props) {
+    if (!props) {
+        props = celix_properties_create();
+    }
+    if (!props) {
+        return CELIX_ENOMEM;
+    }
 
-       if (properties_get(dictionary, (char *) CELIX_FRAMEWORK_SERVICE_NAME) 
== NULL) {
-               properties_set(dictionary, (char *) 
CELIX_FRAMEWORK_SERVICE_NAME, registration->className);
-       }
+    celix_status_t status = celix_properties_setLong(props, 
CELIX_FRAMEWORK_SERVICE_ID, registration->serviceId);
+    CELIX_DO_IF(status, status = celix_properties_set(props, 
CELIX_FRAMEWORK_SERVICE_NAME, registration->className));
 
-       registration->properties = dictionary;
+    if (status == CELIX_SUCCESS) {
+        registration->properties = props;
+    } else {
+        celix_err_push("Cannot initialize service registration properties");
+        properties_destroy(props);
+    }
 
-       return CELIX_SUCCESS;
+    return CELIX_SUCCESS;
 }
 
 void serviceRegistration_invalidate(service_registration_pt registration) {
diff --git a/libs/framework/src/service_registration_private.h 
b/libs/framework/src/service_registration_private.h
index bda962e6..0e896df2 100644
--- a/libs/framework/src/service_registration_private.h
+++ b/libs/framework/src/service_registration_private.h
@@ -35,25 +35,25 @@ struct serviceRegistration {
     struct celix_ref refCount;
     registry_callback_t callback; // read-only
 
-       char * className; // read-only
-       bundle_pt bundle; // read-only
-       properties_pt properties; // read-only
-       unsigned long serviceId; // read-only
+    char* className;          // read-only
+    bundle_pt bundle;         // read-only
+    properties_pt properties; // read-only
+    long serviceId;           // read-only
 
     bool isUnregistering;
 
     enum celix_service_type svcType; // read-only
     union {
-        const void * svcObj;
+        const void* svcObj;
         service_factory_pt deprecatedFactory;
-        celix_service_factory_t *factory;
+        celix_service_factory_t* factory;
     };
 
     celix_thread_rwlock_t lock; // protects the service object
 };
 
-service_registration_pt serviceRegistration_create(registry_callback_t 
callback, bundle_pt bundle, const char* serviceName, unsigned long serviceId, 
const void * serviceObject, properties_pt dictionary);
-service_registration_pt 
serviceRegistration_createServiceFactory(registry_callback_t callback, 
bundle_pt bundle, const char* serviceName, unsigned long serviceId, const void 
* serviceObject, properties_pt dictionary);
+service_registration_pt serviceRegistration_create(registry_callback_t 
callback, bundle_pt bundle, const char* serviceName, long serviceId, const void 
* serviceObject, properties_pt dictionary);
+service_registration_pt 
serviceRegistration_createServiceFactory(registry_callback_t callback, 
bundle_pt bundle, const char* serviceName, long serviceId, const void * 
serviceObject, properties_pt dictionary);
 
 void serviceRegistration_retain(service_registration_pt registration);
 void serviceRegistration_release(service_registration_pt registration);
diff --git a/libs/utils/gtest/src/FilterTestSuite.cc 
b/libs/utils/gtest/src/FilterTestSuite.cc
index 2dd171e2..0a493f73 100644
--- a/libs/utils/gtest/src/FilterTestSuite.cc
+++ b/libs/utils/gtest/src/FilterTestSuite.cc
@@ -435,6 +435,33 @@ TEST_F(FilterTestSuite, MissingOperandCreateTest) {
     ASSERT_TRUE(filter == nullptr);
 }
 
+TEST_F(FilterTestSuite, TypedPropertiesAndFilterTest) {
+    celix_autoptr(celix_properties_t) props = celix_properties_create();
+    celix_autoptr(celix_version_t) version = 
celix_version_createVersionFromString("1.2.3");
+    celix_properties_set(props, "str", "test");
+    celix_properties_setLong(props, "long", 1L);
+    celix_properties_setDouble(props, "double", 0.0);
+    celix_properties_setBool(props, "bool", true);
+    celix_properties_setVersion(props, "version", version);
+
+    celix_autoptr(celix_filter_t) filter1 =
+        
celix_filter_create("(&(str=test)(long=1)(double=0.0)(bool=true)(version=1.2.3))");
+    EXPECT_TRUE(celix_filter_match(filter1, props));
+
+    celix_autoptr(celix_filter_t) filter2 =
+        
celix_filter_create("(&(str>tes)(long>0)(double>-1.0)(bool>false)(version>1.0.0))");
+    EXPECT_TRUE(celix_filter_match(filter2, props));
+
+    celix_autoptr(celix_filter_t) filter3 =
+                
celix_filter_create("(&(str<tesu)(long<2)(double<1.0)(bool<=true)(version<2.0.0))");
+    EXPECT_TRUE(celix_filter_match(filter3, props));
+
+    celix_autoptr(celix_filter_t) filter4 =
+            
celix_filter_create("(&(str!=test)(long!=1)(double!=0.0)(bool!=true)(version!=1.2.3))");
+    EXPECT_FALSE(celix_filter_match(filter4, props));
+}
+
+
 TEST_F(FilterTestSuite, SubStringTest) {
     celix_autoptr(celix_properties_t) props = celix_properties_create();
     celix_properties_set(props, "test", "John Bob Doe");
diff --git a/libs/utils/src/filter.c b/libs/utils/src/filter.c
index f8eced27..e1a20652 100644
--- a/libs/utils/src/filter.c
+++ b/libs/utils/src/filter.c
@@ -32,11 +32,19 @@
 #include "celix_version.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;
 };
@@ -51,8 +59,8 @@ 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 void filter_skipWhiteSpace(char * filterString, int * pos) {
-    int length;
+static void filter_skipWhiteSpace(char* filterString, int* pos) {
+    size_t length;
     for (length = strlen(filterString); (*pos < length) && 
isspace(filterString[*pos]);) {
         (*pos)++;
     }
@@ -111,8 +119,10 @@ static celix_filter_t * filter_parseFilterComp(char * 
filterString, int * pos) {
             (*pos)++;
             return filter_parseNot(filterString, pos);
         }
+        default: {
+            return filter_parseItem(filterString, pos);
+        }
     }
-    return filter_parseItem(filterString, pos);
 }
 
 static celix_filter_t * filter_parseAndOrOr(char * filterString, 
celix_filter_operand_t andOrOr, int * pos) {
@@ -210,7 +220,6 @@ static celix_filter_t * filter_parseItem(char * 
filterString, int * pos) {
                 filter->value = filter_parseValue(filterString, pos);
                 return filter;
             }
-            break;
         }
         case '<': {
             if (filterString[*pos + 1] == '=') {
@@ -229,7 +238,6 @@ static celix_filter_t * filter_parseItem(char * 
filterString, int * pos) {
                 filter->value = filter_parseValue(filterString, pos);
                 return filter;
             }
-            break;
         }
         case '=': {
             celix_filter_t * filter = NULL;
@@ -280,7 +288,7 @@ static char * filter_parseAttr(char * filterString, int * 
pos) {
     char c;
     int begin = *pos;
     int end = *pos;
-    int length = 0;
+    int length;
 
     filter_skipWhiteSpace(filterString, pos);
     c = filterString[*pos];
@@ -443,6 +451,7 @@ static celix_status_t celix_filter_compile(celix_filter_t* 
filter) {
             filter->internal->longValue = 
celix_utils_convertStringToLong(filter->value, 0, 
&filter->internal->convertedToLong);
             filter->internal->doubleValue = 
celix_utils_convertStringToDouble(filter->value, 0.0, 
&filter->internal->convertedToDouble);
             filter->internal->versionValue = 
celix_utils_convertStringToVersion(filter->value, NULL, 
&filter->internal->convertedToVersion);
+            filter->internal->boolValue = 
celix_utils_convertStringToBool(filter->value, false, 
&filter->internal->convertedToBool);
         }
     }
 
@@ -459,7 +468,7 @@ static celix_status_t celix_filter_compile(celix_filter_t* 
filter) {
     return CELIX_SUCCESS;
 }
 
-celix_status_t filter_match(celix_filter_t * filter, celix_properties_t 
*properties, bool *out) {
+celix_status_t filter_match(celix_filter_t* filter, celix_properties_t* 
properties, bool* out) {
     bool result = celix_filter_match(filter, properties);
     if (out != NULL) {
         *out = result;
@@ -467,64 +476,95 @@ celix_status_t filter_match(celix_filter_t * filter, 
celix_properties_t *propert
     return CELIX_SUCCESS;
 }
 
-static int celix_filter_compareAttributeValue(const celix_filter_t* filter, 
const celix_properties_entry_t* entry) {
-    const char* propertyValue = entry->value;
-    if (!filter->internal->convertedToLong && 
!filter->internal->convertedToDouble && !filter->internal->convertedToVersion) {
-        return strcmp(propertyValue, filter->value);
+static int celix_filter_cmpLong(long a, long b) {
+    if (a < b) {
+        return -1;
+    } else if (a > b) {
+        return 1;
+    } else {
+        return 0;
     }
+}
 
-    if (filter->internal->convertedToLong) {
-        bool propertyValueIsLong = false;
-        long value = celix_utils_convertStringToLong(propertyValue, 0, 
&propertyValueIsLong);
-        if (propertyValueIsLong) {
-            if (value < filter->internal->longValue)
-                return -1;
-            else if (value > filter->internal->longValue)
-                return 1;
-            else
-                return 0;
-        }
+static int celix_filter_cmpDouble(double a, double b) {
+    if (a < b) {
+        return -1;
+    } else if (a > b) {
+        return 1;
+    } else {
+        return 0;
+    }
+}
+
+static int celix_filter_cmpBool(bool a, bool b) {
+    if (a == b) {
+        return 0;
+    } else if (a) {
+        return 1;
+    } else {
+        return -1;
     }
+}
 
-    if (filter->internal->convertedToDouble) {
-        bool propertyValueIsDouble = false;
-        double value = celix_utils_convertStringToDouble(propertyValue, 0.0, 
&propertyValueIsDouble);
-        if (propertyValueIsDouble) {
-            if (value < filter->internal->doubleValue) {
-                return -1;
-            } else if (value > filter->internal->doubleValue) {
-                return 1;
-            } else {
-                return 0;
-            }
-        }
+static int celix_filter_compareAttributeValue(const celix_filter_t* filter, 
const celix_properties_entry_t* entry) {
+    // not converted, fallback on string compare
+    if (!filter->internal->convertedToLong && 
!filter->internal->convertedToDouble &&
+        !filter->internal->convertedToBool && 
!filter->internal->convertedToVersion) {
+        return strcmp(entry->value, filter->value);
     }
 
-    if (filter->internal->convertedToVersion) {
-        bool propertyValueIsVersion = false;
-        celix_version_t *value = 
celix_utils_convertStringToVersion(propertyValue, NULL, 
&propertyValueIsVersion);
-        if (propertyValueIsVersion) {
-            int cmp = celix_version_compareTo(value, 
filter->internal->versionValue);
-            celix_version_destroy(value);
+    //compare typed values
+    if (filter->internal->convertedToLong && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_LONG) {
+        return celix_filter_cmpLong(entry->typed.longValue, 
filter->internal->longValue);
+    } else if (filter->internal->convertedToDouble && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) {
+        return celix_filter_cmpDouble(entry->typed.doubleValue, 
filter->internal->doubleValue);
+    } else if (filter->internal->convertedToBool && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_BOOL) {
+        return celix_filter_cmpBool(entry->typed.boolValue, 
filter->internal->boolValue);
+    } else if (filter->internal->convertedToVersion && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+        return celix_version_compareTo(entry->typed.versionValue, 
filter->internal->versionValue);
+    }
+
+    //check if the property string value can be converted to the filter value 
type
+    bool propertyConverted;
+    if (filter->internal->convertedToLong) {
+        long val = celix_utils_convertStringToLong(entry->value, 0, 
&propertyConverted);
+        if (propertyConverted) {
+            return celix_filter_cmpLong(val, filter->internal->longValue);
+        }
+    } else if (filter->internal->convertedToDouble) {
+        double val = celix_utils_convertStringToDouble(entry->value, 0.0, 
&propertyConverted);
+        if (propertyConverted) {
+            return celix_filter_cmpDouble(val, filter->internal->doubleValue);
+        }
+    } else if (filter->internal->convertedToBool) {
+        bool val = celix_utils_convertStringToBool(entry->value, false, 
&propertyConverted);
+        if (propertyConverted) {
+            return celix_filter_cmpBool(val, filter->internal->boolValue);
+        }
+    } else if (filter->internal->convertedToVersion) {
+        celix_version_t* val = 
celix_utils_convertStringToVersion(entry->value, NULL, &propertyConverted);
+        if (propertyConverted) {
+            int cmp = celix_version_compareTo(val, 
filter->internal->versionValue);
+            celix_version_destroy(val);
             return cmp;
         }
     }
 
     //fallback on string compare
-    return strcmp(propertyValue, filter->value);
+    return strcmp(entry->value, filter->value);
 }
 
 static bool filter_compareSubString(const celix_filter_t* filter, const 
celix_properties_entry_t* entry) {
     const char* propertyValue = entry->value;
-    int pos = 0;
+    size_t pos = 0;
     int size = celix_arrayList_size(filter->children);
-    for (int i = 0; i < size; i++) {
-        char * substr = (char *) celix_arrayList_get(filter->children, i);
+    for (size_t i = 0; i < size; i++) {
+        char * substr = (char *) celix_arrayList_get(filter->children, (int)i);
 
         if (i + 1 < size) {
             if (substr == NULL) {
                 unsigned int index;
-                char * substr2 = (char *) 
celix_arrayList_get(filter->children, i + 1);
+                char * substr2 = (char *) 
celix_arrayList_get(filter->children, (int)(i + 1));
                 if (substr2 == NULL) {
                         continue;
                 }
@@ -551,8 +591,8 @@ static bool filter_compareSubString(const celix_filter_t* 
filter, const celix_pr
                 free(region);
             }
         } else {
-            unsigned int len;
-            int begin;
+            size_t len;
+            size_t begin;
 
             if (substr == NULL) {
                 return true;
@@ -590,9 +630,10 @@ static bool celix_filter_matchPropertyEntry(const 
celix_filter_t* filter, const
         case CELIX_FILTER_OPERAND_OR:
         case CELIX_FILTER_OPERAND_PRESENT:
         default:
-            assert(false); //should not reach here
+            //LCOV EXCL START
+            return false; //should not happen
+            //LCOV EXCL STOP
     }
-    return false;
 }
 
 celix_status_t filter_getString(celix_filter_t * filter, const char 
**filterStr) {
@@ -615,7 +656,6 @@ celix_filter_t* celix_filter_create(const char 
*filterString) {
     celix_autofree char* str = celix_utils_strdup(filterString);
     if (!str) {
         celix_err_push("Failed to create filter string");
-        // TODO test error
         return NULL;
     }
 
@@ -735,8 +775,7 @@ bool celix_filter_match(const celix_filter_t* filter, const 
celix_properties_t*
     }
 
     // LCOV_EXCL_START
-    assert(false); // should never happen
-    return false;
+    return false;// should not happen
     // LCOV_EXCL_STOP
 }
 
@@ -888,3 +927,5 @@ static bool hasMandatoryNegatedPresenceAttribute(const 
celix_filter_t *filter, c
 bool celix_filter_hasMandatoryNegatedPresenceAttribute(const celix_filter_t 
*filter, const char *attribute) {
     return hasMandatoryNegatedPresenceAttribute(filter, attribute, false, 
false);
 }
+
+//NOLINTEND(misc-no-recursion)

Reply via email to