You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by "PengZheng (via GitHub)" <gi...@apache.org> on 2023/05/06 10:36:36 UTC
[PR] Feature/trim string in place (celix)
PengZheng opened a new pull request, #545:
URL: https://github.com/apache/celix/pull/545
This PR adds deprecated `utils_stringTrim` back as `celix_utils_trimInPlace`, and recover all its proper use throughout the whole code base.
I notice that currently there are two `celix_ei_expect_celix_utils_trim` in `RsaShmUnitTestSuite`.
Please have review the rsa_shm part (including its testing). @xuzhenbao
--
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
Re: [PR] Feature/trim string in place (celix)
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #545:
URL: https://github.com/apache/celix/pull/545#issuecomment-1537113946
## [Codecov](https://app.codecov.io/gh/apache/celix/pull/545?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 [#545](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0be6a84) into [master](https://app.codecov.io/gh/apache/celix/commit/4281d76581bb6c4ea9c869c02c1cef732f1e055b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4281d76) will **decrease** coverage by `0.04%`.
> The diff coverage is `92.00%`.
```diff
@@ Coverage Diff @@
## master #545 +/- ##
==========================================
- Coverage 77.48% 77.44% -0.04%
==========================================
Files 226 226
Lines 34642 34622 -20
==========================================
- Hits 26841 26812 -29
- Misses 7801 7810 +9
```
| [Impacted Files](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...es/pubsub/pubsub\_admin\_tcp/src/pubsub\_tcp\_common.c](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX2FkbWluX3RjcC9zcmMvcHVic3ViX3RjcF9jb21tb24uYw==) | `0.00% <0.00%> (ø)` | |
| [libs/framework/src/manifest\_parser.c](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy9mcmFtZXdvcmsvc3JjL21hbmlmZXN0X3BhcnNlci5j) | `32.06% <0.00%> (ø)` | |
| [...e\_service\_admin\_dfi/src/remote\_service\_admin\_dfi.c](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9yZW1vdGVfc2VydmljZXMvcmVtb3RlX3NlcnZpY2VfYWRtaW5fZGZpL3NyYy9yZW1vdGVfc2VydmljZV9hZG1pbl9kZmkuYw==) | `84.82% <100.00%> (ø)` | |
| [...n\_shm\_v2/rsa\_shm/src/rsa\_shm\_export\_registration.c](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9yZW1vdGVfc2VydmljZXMvcmVtb3RlX3NlcnZpY2VfYWRtaW5fc2htX3YyL3JzYV9zaG0vc3JjL3JzYV9zaG1fZXhwb3J0X3JlZ2lzdHJhdGlvbi5j) | `100.00% <100.00%> (ø)` | |
| [...te\_service\_admin\_shm\_v2/rsa\_shm/src/rsa\_shm\_impl.c](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9yZW1vdGVfc2VydmljZXMvcmVtb3RlX3NlcnZpY2VfYWRtaW5fc2htX3YyL3JzYV9zaG0vc3JjL3JzYV9zaG1faW1wbC5j) | `96.78% <100.00%> (-0.12%)` | :arrow_down: |
| [...n\_shm\_v2/rsa\_shm/src/rsa\_shm\_import\_registration.c](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9yZW1vdGVfc2VydmljZXMvcmVtb3RlX3NlcnZpY2VfYWRtaW5fc2htX3YyL3JzYV9zaG0vc3JjL3JzYV9zaG1faW1wb3J0X3JlZ2lzdHJhdGlvbi5j) | `98.19% <100.00%> (-0.05%)` | :arrow_down: |
| [bundles/shell/shell\_tui/src/shell\_tui.c](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9zaGVsbC9zaGVsbF90dWkvc3JjL3NoZWxsX3R1aS5j) | `79.29% <100.00%> (ø)` | |
| [libs/framework/src/module.c](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy9mcmFtZXdvcmsvc3JjL21vZHVsZS5j) | `74.36% <100.00%> (ø)` | |
| [libs/utils/src/celix\_convert\_utils.c](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy91dGlscy9zcmMvY2VsaXhfY29udmVydF91dGlscy5j) | `100.00% <100.00%> (ø)` | |
| [libs/utils/src/properties.c](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy91dGlscy9zcmMvcHJvcGVydGllcy5j) | `80.90% <100.00%> (ø)` | |
| ... and [1 more](https://app.codecov.io/gh/apache/celix/pull/545?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
... and [4 files with indirect coverage changes](https://app.codecov.io/gh/apache/celix/pull/545/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
Re: [PR] Feature/trim string in place (celix)
Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on code in PR #545:
URL: https://github.com/apache/celix/pull/545#discussion_r1186681809
##########
libs/utils/src/celix_convert_utils.c:
##########
@@ -114,7 +114,7 @@ celix_version_t* celix_utils_convertStringToVersion(const char* val, const celix
if (firstDot != NULL && lastDot != NULL && firstDot != lastDot) {
char buf[64];
char* valCopy = celix_utils_writeOrCreateString(buf, sizeof(buf), "%s", val);
- char *trimmed = utils_stringTrim(valCopy);
+ char *trimmed = celix_utils_trimInPlace(valCopy);
Review Comment:
nice, I think this also fixes an bug. Because in the old situation the trimmed copy got freed in `celix_utils_freeStringIfNotEqual`, but if `celix_utils_writeOrCreateString` used a allocation this would leak.
##########
libs/utils/include/celix_utils.h:
##########
@@ -105,6 +105,14 @@ CELIX_UTILS_EXPORT bool celix_utils_containsWhitespace(const char* s);
* Caller is owner of the returned string.
*/
CELIX_UTILS_EXPORT char* celix_utils_trim(const char* string);
+/**
+ * @brief Trims the provided string in place.
+ *
+ * The trim will remove eny leading and trailing whitespaces (' ', '\t', etc based on `isspace`)/
Review Comment:
nitpick: typo "any" instead of "eny"
--
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
Re: [PR] Feature/trim string in place (celix)
Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng merged PR #545:
URL: https://github.com/apache/celix/pull/545
--
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