You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by "pnoltes (via GitHub)" <gi...@apache.org> on 2023/03/31 14:23:24 UTC
[GitHub] [celix] pnoltes opened a new pull request, #505: Feature/parse filter attribute types
pnoltes opened a new pull request, #505:
URL: https://github.com/apache/celix/pull/505
This PR fixes #432, but depends on the celix_convert_utils.h from PR #476
Also refactors filter cpputests to gtests.
--
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-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a diff in pull request #505: Feature/parse filter attribute types
Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on code in PR #505:
URL: https://github.com/apache/celix/pull/505#discussion_r1161891928
##########
libs/utils/src/filter.c:
##########
@@ -554,14 +652,15 @@ celix_filter_t* celix_filter_create(const char *filterString) {
free(filterStr);
} else {
filter->filterStr = filterStr;
+ celix_filter_compile(filter);
Review Comment:
return value check added
--
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-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] PengZheng commented on a diff in pull request #505: Feature/parse filter attribute types
Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #505:
URL: https://github.com/apache/celix/pull/505#discussion_r1160518229
##########
libs/utils/src/filter.c:
##########
@@ -415,6 +469,47 @@ 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 char* propertyValue) {
+ if (!filter->internal->convertedToLong && !filter->internal->convertedToDouble && !filter->internal->convertedToVersion) {
+ return strcmp(propertyValue, filter->value);
+ }
+
+ if (filter->internal->convertedToLong) {
+ bool propertyValueIsLong = false;
+ long value = celix_utils_convertStringToLong(propertyValue, 0, &propertyValueIsLong);
+ if (propertyValueIsLong) {
+ return (int) (value - filter->internal->longValue);
Review Comment:
Integer overflow happens.
--
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-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-commenter commented on pull request #505: Feature/parse filter attribute types
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #505:
URL: https://github.com/apache/celix/pull/505#issuecomment-1492025873
## [Codecov](https://codecov.io/gh/apache/celix/pull/505?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#505](https://codecov.io/gh/apache/celix/pull/505?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2412062) into [master](https://codecov.io/gh/apache/celix/commit/42e9669b77af2767bec097a6d26a04f8a45b66a3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (42e9669) will **decrease** coverage by `0.50%`.
> The diff coverage is `96.58%`.
> :exclamation: Current head 2412062 differs from pull request most recent head 3493d6a. Consider uploading reports for the commit 3493d6a to get more accurate results
```diff
@@ Coverage Diff @@
## master #505 +/- ##
==========================================
- Coverage 76.48% 75.99% -0.50%
==========================================
Files 226 226
Lines 34133 34240 +107
==========================================
- Hits 26108 26021 -87
- Misses 8025 8219 +194
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/505?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [libs/utils/src/filter.c](https://codecov.io/gh/apache/celix/pull/505?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy91dGlscy9zcmMvZmlsdGVyLmM=) | `88.29% <96.58%> (-7.28%)` | :arrow_down: |
... and [6 files with indirect coverage changes](https://codecov.io/gh/apache/celix/pull/505/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] PengZheng commented on a diff in pull request #505: Feature/parse filter attribute types
Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #505:
URL: https://github.com/apache/celix/pull/505#discussion_r1160510169
##########
libs/utils/src/filter.c:
##########
@@ -407,6 +418,49 @@ static celix_array_list_t* filter_parseSubstring(char * filterString, int * pos)
return operands;
}
+static bool celix_filter_isCompareOperand(celix_filter_operand_t operand) {
+ return operand == CELIX_FILTER_OPERAND_EQUAL ||
+ operand == CELIX_FILTER_OPERAND_GREATER ||
+ operand == CELIX_FILTER_OPERAND_LESS ||
+ operand == CELIX_FILTER_OPERAND_GREATEREQUAL ||
+ operand == CELIX_FILTER_OPERAND_LESSEQUAL;
+}
+
+
+static bool celix_filter_hasFilterChildren(celix_filter_t* filter) {
+ return filter->operand == CELIX_FILTER_OPERAND_AND ||
+ filter->operand == CELIX_FILTER_OPERAND_OR ||
+ filter->operand == CELIX_FILTER_OPERAND_NOT;
+}
+
+/**
+ * Compiles the filter, so that the attribute values are converted to the typed values if possible.
+ */
+static celix_status_t celix_filter_compile(celix_filter_t* filter) {
+ if (celix_filter_isCompareOperand(filter->operand)) {
+ filter->internal = malloc(sizeof(*filter->internal));
+ if (filter->internal == NULL) {
+ return CELIX_ENOMEM;
+ } else {
+ 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);
Review Comment:
`celix_utils_convertStringToVersion` is flawed, see #512. It's fixed by #513.
--
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-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes merged pull request #505: Feature/parse filter attribute types
Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes merged PR #505:
URL: https://github.com/apache/celix/pull/505
--
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-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] PengZheng commented on pull request #505: Feature/parse filter attribute types
Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on PR #505:
URL: https://github.com/apache/celix/pull/505#issuecomment-1500199358
It seems that some corner cases remain unresolved, e.g. `man 3 strtod`
> An infinity is either "INF" or "INFINITY", disregarding case.
--
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-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] PengZheng commented on a diff in pull request #505: Feature/parse filter attribute types
Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #505:
URL: https://github.com/apache/celix/pull/505#discussion_r1160510169
##########
libs/utils/src/filter.c:
##########
@@ -407,6 +418,49 @@ static celix_array_list_t* filter_parseSubstring(char * filterString, int * pos)
return operands;
}
+static bool celix_filter_isCompareOperand(celix_filter_operand_t operand) {
+ return operand == CELIX_FILTER_OPERAND_EQUAL ||
+ operand == CELIX_FILTER_OPERAND_GREATER ||
+ operand == CELIX_FILTER_OPERAND_LESS ||
+ operand == CELIX_FILTER_OPERAND_GREATEREQUAL ||
+ operand == CELIX_FILTER_OPERAND_LESSEQUAL;
+}
+
+
+static bool celix_filter_hasFilterChildren(celix_filter_t* filter) {
+ return filter->operand == CELIX_FILTER_OPERAND_AND ||
+ filter->operand == CELIX_FILTER_OPERAND_OR ||
+ filter->operand == CELIX_FILTER_OPERAND_NOT;
+}
+
+/**
+ * Compiles the filter, so that the attribute values are converted to the typed values if possible.
+ */
+static celix_status_t celix_filter_compile(celix_filter_t* filter) {
+ if (celix_filter_isCompareOperand(filter->operand)) {
+ filter->internal = malloc(sizeof(*filter->internal));
+ if (filter->internal == NULL) {
+ return CELIX_ENOMEM;
+ } else {
+ 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);
Review Comment:
`celix_utils_convertStringToVersion` is flawed, see #512
--
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-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] PengZheng commented on a diff in pull request #505: Feature/parse filter attribute types
Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #505:
URL: https://github.com/apache/celix/pull/505#discussion_r1160684767
##########
libs/utils/src/filter.c:
##########
@@ -407,6 +418,49 @@ static celix_array_list_t* filter_parseSubstring(char * filterString, int * pos)
return operands;
}
+static bool celix_filter_isCompareOperand(celix_filter_operand_t operand) {
+ return operand == CELIX_FILTER_OPERAND_EQUAL ||
+ operand == CELIX_FILTER_OPERAND_GREATER ||
+ operand == CELIX_FILTER_OPERAND_LESS ||
+ operand == CELIX_FILTER_OPERAND_GREATEREQUAL ||
+ operand == CELIX_FILTER_OPERAND_LESSEQUAL;
+}
+
+
+static bool celix_filter_hasFilterChildren(celix_filter_t* filter) {
+ return filter->operand == CELIX_FILTER_OPERAND_AND ||
+ filter->operand == CELIX_FILTER_OPERAND_OR ||
+ filter->operand == CELIX_FILTER_OPERAND_NOT;
+}
+
+/**
+ * Compiles the filter, so that the attribute values are converted to the typed values if possible.
+ */
+static celix_status_t celix_filter_compile(celix_filter_t* filter) {
+ if (celix_filter_isCompareOperand(filter->operand)) {
+ filter->internal = malloc(sizeof(*filter->internal));
+ if (filter->internal == NULL) {
+ return CELIX_ENOMEM;
+ } else {
+ 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);
Review Comment:
`celix_utils_convertStringToDouble` and `celix_utils_convertStringToLong` are also flawed. They are fixed by the following:
https://github.com/apache/celix/pull/514
--
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-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] PengZheng commented on a diff in pull request #505: Feature/parse filter attribute types
Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #505:
URL: https://github.com/apache/celix/pull/505#discussion_r1160499103
##########
libs/utils/src/filter.c:
##########
@@ -554,14 +652,15 @@ celix_filter_t* celix_filter_create(const char *filterString) {
free(filterStr);
} else {
filter->filterStr = filterStr;
+ celix_filter_compile(filter);
Review Comment:
Return value is not checked.
--
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-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org