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


##########
libs/utils/include/celix_filter.h:
##########
@@ -44,63 +89,121 @@ typedef enum celix_filter_operand_enum
     CELIX_FILTER_OPERAND_NOT,
 } celix_filter_operand_t;
 
+/**
+ * @brief Internal opaque struct for internal use only.
+ */
+typedef struct celix_filter_internal celix_filter_internal_t; // opaque struct 
for internal use only
 
-typedef struct celix_filter_internal celix_filter_internal_t; //opaque struct 
for internal use only
-
-typedef struct celix_filter_struct celix_filter_t;
-
-struct celix_filter_struct {
-    celix_filter_operand_t operand;
-    const char *attribute; //NULL for operands AND, OR ot NOT
-    const char *value; //NULL for operands AND, OR or NOT NOT
-    const char *filterStr;
+/**
+ * @brief The Apache Celix filter struct.
+ */
+typedef struct celix_filter_struct {
+    celix_filter_operand_t operand; /**< The filter operand. */
+    const char* attribute;          /**< The filter attribute; NULL for 
operands `AND`, `OR` or `NOT`. */
+    const char* value;              /**< The filter value; NULL for operands 
`AND`, `OR` or `NOT`. */
+    const char* filterStr;          /**< The filter string representation. */
 
-    //type is celix_filter_t* for AND, OR and NOT operator and char* for 
SUBSTRING
-    //for other operands children is NULL
-    celix_array_list_t *children;
+    celix_array_list_t* children; /**< The filter children; only valid if the 
operand is not `AND`, `OR` or `NOT` else
+                                     the value is NULL. */
 
-    celix_filter_internal_t* internal; //for internal use only
-};
+    celix_filter_internal_t* internal; /**< Internal use only. */
+} celix_filter_t;
 
-CELIX_UTILS_EXPORT celix_filter_t* celix_filter_create(const char *filterStr);
+/**
+ * @brief Create a filter based on the provided filter string.
+ *
+ * If the return status is NULL, an error message is logged to celix_err.
+ *
+ * @param filterStr The filter string. If NULL or "" a match-all "(|)" filter 
is created.
+ * @return The created filter or NULL if the filter string is invalid.
+ */
+CELIX_UTILS_EXPORT celix_filter_t* celix_filter_create(const char* filterStr);
 
-CELIX_UTILS_EXPORT void celix_filter_destroy(celix_filter_t *filter);
+/**
+ * @brief Destroy the provided filter. Ignores NULL values.
+ */
+CELIX_UTILS_EXPORT void celix_filter_destroy(celix_filter_t* filter);
 
+/**
+ * @brief Add support for `celix_autoptr`.
+ */
 CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(celix_filter_t, celix_filter_destroy)
 
-CELIX_UTILS_EXPORT bool celix_filter_match(const celix_filter_t *filter, const 
celix_properties_t* props);
+/**
+ * @brief Check whether the provided filter matches the provided properties.
+ * @param[in] filter The filter.
+ * @param[in] props The properties.
+ * @return True if the filter matches the properties, false otherwise. If 
filter is NULL always returns true and
+ *         if props is NULL, the result is the same as if an empty properties 
set was provided.
+ */
+CELIX_UTILS_EXPORT bool celix_filter_match(const celix_filter_t* filter, const 
celix_properties_t* props);
 
-CELIX_UTILS_EXPORT bool celix_filter_matchFilter(const celix_filter_t 
*filter1, const celix_filter_t *filter2);
+/**
+ * @brief Check whether the 2 filters are equal.
+ *
+ * Note that the equals is done based on the parsed filter string and not on 
the provided filter strings.
+ * So the filters parsed from the strings "(key=value)" and " (key=value)" are 
equal.
+ *
+ * @param[in] filter1 The first filter.
+ * @param[in] filter2 The second filter.
+ * @return True if the filters are equal, false otherwise.
+ */
+CELIX_UTILS_EXPORT bool celix_filter_equals(const celix_filter_t* filter1, 
const celix_filter_t* filter2);
 
-CELIX_UTILS_EXPORT const char* celix_filter_getFilterString(const 
celix_filter_t *filter);
+/**
+ * @brief Get the filter string representation.
+ *
+ * @param[in] filter The filter.
+ * @return The filter string representation. The returned string is owned by 
the filter, must not be freed and is only
+ *         valid as long as the filter is valid.
+ */
+CELIX_UTILS_EXPORT const char* celix_filter_getFilterString(const 
celix_filter_t* filter);
 
 /**
- * Find the filter attribute.
- * @return The attribute value or NULL if the attribute is not found.
+ * @brief Find the filter attribute.
+ *
+ * @param[in] filter The filter.
+ * @param[in] attribute The attribute to find.
+ * @return The found attribute value or NULL if the attribute is not found. 
The returned string is owned by the filter,
+ *         must not be freed and is only valid as long as the filter is valid.
  */
-CELIX_UTILS_EXPORT const char* celix_filter_findAttribute(const celix_filter_t 
*filter, const char *attribute);
+CELIX_UTILS_EXPORT const char* celix_filter_findAttribute(const 
celix_filter_t* filter, const char* attribute);
 
 /**
- * Check whether the filter indicates the mandatory presence of an attribute 
with a specific value for the provided attribute key.
+ * @brief Check whether the filter indicates the mandatory presence of an 
attribute with a specific value for the
+ * provided attribute key.
  *
  * Example:
  *   using this function for attribute key "key1" on filter "(key1=value1)" 
yields true.
  *   using this function for attribute key "key1" on filter "(!(key1=value1))" 
yields false.
  *   using this function for attribute key "key1" on filter "(key1>=value1)" 
yields false.
  *   using this function for attribute key "key1" on filter 
"(|(key1=value1)(key2=value2))" yields false.
-*/
-CELIX_UTILS_EXPORT bool celix_filter_hasMandatoryEqualsValueAttribute(const 
celix_filter_t *filter, const char *attribute);
+ *
+ * @param[in] filter The filter.
+ * @param[in] attribute The attribute to check.
+ * @return True if the filter indicates the mandatory presence of an attribute 
with a specific value for the provided
+ *         attribute key, false otherwise.
+ */
+CELIX_UTILS_EXPORT bool celix_filter_hasMandatoryEqualsValueAttribute(const 
celix_filter_t* filter,

Review Comment:
   The comment says `using this function for attribute key "key1" on filter 
"(|(key1=value1)(key2=value2))" yields false`, which seems inconsistent with 
`EXPECT_TRUE(filter2.hasMandatoryEqualsValueAttribute("key3"))` for `filter2` I 
gave above. Note that `(!(&(key=value)(!(key3=value))))` is equivalent to 
`(|(!(key=value))(key3=value))`.
   
   



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