You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2023/01/16 12:38:32 UTC

[GitHub] [celix] PengZheng opened a new pull request, #473: Add basic error injector mechanism.

PengZheng opened a new pull request, #473:
URL: https://github.com/apache/celix/pull/473

   It improves the mocking malloc / realloc introduced by #459 in the following ways:
   
   - It can be used together with various sanitizers. 
   - The caller's address can be used to specify which call to inject error into.
   
   In my day-time job, I use it to inject error successfully into various libraries, e.g. libc, openssl, sqlite, libuv.


-- 
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 #473: Add basic error injector mechanism.

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1084872160


##########
misc/error_injector/celix_properties/celix_properties_ei.cpp:
##########
@@ -0,0 +1,29 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#include <celix_properties_ei.h>

Review Comment:
   Oops! I should have followed Celix's existing convention, and it will be corrected.
   
   In John Lakos's Large Scale C++, it is suggested that angle brackets instead of double quotes should be used exclusively in include directives. I follow his methods in my day-time job. However, existing convention should always take priority.



-- 
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 #473: Add basic error injector mechanism.

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1092707788


##########
libs/error_injector/README.md:
##########
@@ -0,0 +1,81 @@
+---
+title: Error Injector
+---
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+   
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+# Celix Error Injector

Review Comment:
   I'll add a table.



-- 
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 pull request #473: Add basic error injector mechanism.

Posted by GitBox <gi...@apache.org>.
pnoltes commented on PR #473:
URL: https://github.com/apache/celix/pull/473#issuecomment-1384306431

   Nice, this cleans up the test code. I will have a look into this and see if I can also understand the mechanism behind this. 


-- 
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 #473: Add basic error injector mechanism.

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #473:
URL: https://github.com/apache/celix/pull/473#issuecomment-1385482017

   # [Codecov](https://codecov.io/gh/apache/celix/pull/473?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 [#473](https://codecov.io/gh/apache/celix/pull/473?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (94bab53) into [master](https://codecov.io/gh/apache/celix/commit/9d786d16610fce7c461005fc802e30f443202486?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9d786d1) will **decrease** coverage by `0.51%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #473      +/-   ##
   ==========================================
   - Coverage   75.57%   75.07%   -0.51%     
   ==========================================
     Files         216      216              
     Lines       32122    32142      +20     
   ==========================================
   - Hits        24275    24129     -146     
   - Misses       7847     8013     +166     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/celix/pull/473?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...b/pubsub\_admin\_zmq/src/pubsub\_zmq\_topic\_receiver.c](https://codecov.io/gh/apache/celix/pull/473?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX2FkbWluX3ptcS9zcmMvcHVic3ViX3ptcV90b3BpY19yZWNlaXZlci5j) | `78.97% <100.00%> (+0.19%)` | :arrow_up: |
   | [...sub/pubsub\_admin\_zmq/src/pubsub\_zmq\_topic\_sender.c](https://codecov.io/gh/apache/celix/pull/473?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX2FkbWluX3ptcS9zcmMvcHVic3ViX3ptcV90b3BpY19zZW5kZXIuYw==) | `84.58% <100.00%> (-0.84%)` | :arrow_down: |
   | [...sub\_protocol\_lib/src/pubsub\_wire\_protocol\_common.c](https://codecov.io/gh/apache/celix/pull/473?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3Byb3RvY29sL3B1YnN1Yl9wcm90b2NvbF9saWIvc3JjL3B1YnN1Yl93aXJlX3Byb3RvY29sX2NvbW1vbi5j) | `87.33% <100.00%> (+1.00%)` | :arrow_up: |
   | [...b\_protocol\_wire\_v1/src/pubsub\_wire\_protocol\_impl.c](https://codecov.io/gh/apache/celix/pull/473?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3Byb3RvY29sL3B1YnN1Yl9wcm90b2NvbF93aXJlX3YxL3NyYy9wdWJzdWJfd2lyZV9wcm90b2NvbF9pbXBsLmM=) | `98.85% <100.00%> (+0.08%)` | :arrow_up: |
   | [...ubsub\_serializer\_json/src/pubsub\_serializer\_impl.c](https://codecov.io/gh/apache/celix/pull/473?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NlcmlhbGl6ZXJfanNvbi9zcmMvcHVic3ViX3NlcmlhbGl6ZXJfaW1wbC5j) | `41.66% <0.00%> (-30.42%)` | :arrow_down: |
   | [...ubsub\_admin\_udp\_mc/src/pubsub\_udpmc\_topic\_sender.c](https://codecov.io/gh/apache/celix/pull/473?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX2FkbWluX3VkcF9tYy9zcmMvcHVic3ViX3VkcG1jX3RvcGljX3NlbmRlci5j) | `55.95% <0.00%> (-27.39%)` | :arrow_down: |
   | [bundles/pubsub/pubsub\_admin\_udp\_mc/src/large\_udp.c](https://codecov.io/gh/apache/celix/pull/473?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX2FkbWluX3VkcF9tYy9zcmMvbGFyZ2VfdWRwLmM=) | `10.81% <0.00%> (-23.79%)` | :arrow_down: |
   | [libs/utils/src/hash\_map.c](https://codecov.io/gh/apache/celix/pull/473?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy91dGlscy9zcmMvaGFzaF9tYXAuYw==) | `96.36% <0.00%> (-0.28%)` | :arrow_down: |
   
   :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 #473: Add basic error injector mechanism.

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1084874743


##########
misc/error_injector/error_injector.h:
##########
@@ -0,0 +1,84 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#ifndef CELIX_ERROR_INJECTOR_H
+#define CELIX_ERROR_INJECTOR_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <stddef.h>
+

Review Comment:
   I think both are needed.



-- 
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 #473: Add basic error injector mechanism.

Posted by GitBox <gi...@apache.org>.
PengZheng commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1082060772


##########
misc/error_injector/malloc/CMakeLists.txt:
##########
@@ -0,0 +1,23 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+add_library(malloc_ei STATIC malloc_ei.cpp)
+
+target_include_directories(malloc_ei PUBLIC ${CMAKE_CURRENT_LIST_DIR})
+target_link_libraries(malloc_ei PUBLIC error_injector)
+# It plays nicely with address sanitizer this way.
+target_link_options(malloc_ei INTERFACE LINKER:--wrap,malloc LINKER:--wrap,realloc LINKER:--wrap,calloc)

Review Comment:
   ```
   ld: unknown option: --wrap
   ```
   
   We need to figure out how to do it on macOS.



-- 
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 #473: Add basic error injector mechanism.

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1084815341


##########
bundles/pubsub/pubsub_protocol/pubsub_protocol_lib/gtest/src/PS_WP_common_ei_tests.cc:
##########
@@ -0,0 +1,233 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+
+
+#include <gtest/gtest.h>
+#include <iostream>
+#include <cstring>
+#include <celix_properties_ei.h>
+#include <malloc_ei.h>
+
+#include "pubsub_wire_protocol_common.h"
+
+class WireProtocolCommonEiTest : public ::testing::Test {
+public:
+    WireProtocolCommonEiTest() = default;
+    ~WireProtocolCommonEiTest() override {
+        celix_ei_expect_realloc(nullptr, 0, nullptr);
+        celix_ei_expect_calloc(nullptr, 0, nullptr);
+        celix_ei_expect_celix_properties_create(nullptr, 0, nullptr);
+    };
+};
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_EncodeMetadataWithNoMemoryLeft) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 0;
+    message.metadata.metadata = celix_properties_create();
+    celix_properties_set(message.metadata.metadata, "key1", "value1");
+
+    //Scenario: No mem with no pre-allocated data
+    //Given (mocked) realloc is forced to return NULL
+    celix_ei_expect_realloc((void *)pubsubProtocol_encodeMetadata, 0, nullptr);
+
+    //When I try to encode a metadata
+    char *data = nullptr;
+    size_t length = 0;
+    size_t contentLength = 0;
+    auto status = pubsubProtocol_encodeMetadata(&message, &data, &length, &contentLength);
+    //Then I expect a failure
+    EXPECT_NE(status, CELIX_SUCCESS);
+
+    //Scenario: No mem with some pre-allocated data
+    //Given a data set with some space
+    data = (char*)malloc(16);
+    length = 16;
+
+    //And (mocked) realloc is forced to return NULL
+    celix_ei_expect_realloc((void *)pubsubProtocol_encodeMetadata, 0, nullptr);
+
+    //When I try to encode a metadata
+    status = pubsubProtocol_encodeMetadata(&message, &data, &length, &contentLength);
+
+    //Then I expect a failure
+    EXPECT_NE(status, CELIX_SUCCESS);
+
+    free(data);
+    celix_properties_destroy(message.metadata.metadata);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DecodeMetadataWithSingleEntries) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 0;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:value1,"); //note 1 entry
+    auto len = strlen(data);
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 1);
+    auto status = pubsubProtocol_decodeMetadata((void*)data, len, &message);
+
+    EXPECT_EQ(status, CELIX_SUCCESS);
+    EXPECT_EQ(1, celix_properties_size(message.metadata.metadata));
+    EXPECT_STREQ("value1", celix_properties_get(message.metadata.metadata, "key1", "not-found"));
+
+    free(data);
+    celix_properties_destroy(message.metadata.metadata);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DencodeMetadataWithMultipleEntries) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 1;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:value1,4:key2,6:value2,6:key111,8:value111,"); //note 3 entries
+    auto len = strlen(data);
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 3);
+    auto status = pubsubProtocol_decodeMetadata((void*)data, len, &message);
+
+    EXPECT_EQ(status, CELIX_SUCCESS);
+    EXPECT_EQ(3, celix_properties_size(message.metadata.metadata));
+    EXPECT_STREQ("value1", celix_properties_get(message.metadata.metadata, "key1", "not-found"));
+    EXPECT_STREQ("value2", celix_properties_get(message.metadata.metadata, "key2", "not-found"));
+    EXPECT_STREQ("value111", celix_properties_get(message.metadata.metadata, "key111", "not-found"));
+
+    free(data);
+    celix_properties_destroy(message.metadata.metadata);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DencodeMetadataWithExtraEntries) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 1;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:value1,4:key2,6:value2,6:key111,8:value111,"); //note 3 entries
+    auto len = strlen(data);
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 2);
+    auto status = pubsubProtocol_decodeMetadata((void*)data, len, &message);
+    // the 3rd entry should be ignored
+    EXPECT_EQ(status, CELIX_SUCCESS);
+    EXPECT_EQ(2, celix_properties_size(message.metadata.metadata));
+    EXPECT_STREQ("value1", celix_properties_get(message.metadata.metadata, "key1", "not-found"));
+    EXPECT_STREQ("value2", celix_properties_get(message.metadata.metadata, "key2", "not-found"));
+    EXPECT_STREQ("not-found", celix_properties_get(message.metadata.metadata, "key111", "not-found"));
+
+    free(data);
+    celix_properties_destroy(message.metadata.metadata);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DencodeMetadataMissingEntries) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 1;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:value1,4:key2,6:value2,6:key111,8:value111,"); //note 3 entries
+    auto len = strlen(data);
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 4);
+    auto status = pubsubProtocol_decodeMetadata((void*)data, len, &message);
+
+    EXPECT_EQ(status, CELIX_INVALID_SYNTAX);
+    EXPECT_EQ(nullptr, message.metadata.metadata);
+
+    free(data);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DecodeMetadataWithSingleEntryWithIncompleteValue) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 0;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:val,"); //note 1 entry with short value
+    auto len = strlen(data);
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 1);
+    auto status = pubsubProtocol_decodeMetadata((void*)data, len, &message);
+
+    EXPECT_EQ(status, CELIX_INVALID_SYNTAX);
+    EXPECT_EQ(nullptr, message.metadata.metadata);
+
+    free(data);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DecodeMetadataTooShort) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 0;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:value1,"); //note 1 entry
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 1);
+    auto status = pubsubProtocol_decodeMetadata((void*)data, 3, &message); // not enough data for `nOfElements`
+
+    EXPECT_EQ(status, CELIX_INVALID_SYNTAX);
+    EXPECT_EQ(nullptr, message.metadata.metadata);
+
+    free(data);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DecodeEmptyMetadata) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 0;
+    message.metadata.metadata = nullptr;
+
+    uint32_t data = 0;
+    auto status = pubsubProtocol_decodeMetadata((void*)&data, 4, &message);
+
+    EXPECT_EQ(status, CELIX_SUCCESS);
+    EXPECT_EQ(nullptr, message.metadata.metadata);
+
+    // incorrect `nOfElements`
+    data = 4;
+    status = pubsubProtocol_decodeMetadata((void*)&data, 4, &message);
+
+    EXPECT_EQ(status, CELIX_INVALID_SYNTAX);
+    EXPECT_EQ(nullptr, message.metadata.metadata);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_NotEnoughMemoryForMultipleEntries) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 1;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:value1,4:key2,6:value2,6:key111,8:value111,"); //note 3 entries
+    auto len = strlen(data);
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 3);
+    for (int i = 0; i < 6; ++i) {
+        celix_ei_expect_calloc((void *)pubsubProtocol_decodeMetadata, 0, nullptr, i+1);

Review Comment:
   Correct. So that errors are injected in all call-sites.



-- 
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 #473: Add basic error injector mechanism.

Posted by GitBox <gi...@apache.org>.
PengZheng commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1071862064


##########
misc/error_injector/error_injector.h:
##########
@@ -0,0 +1,86 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#ifndef CELIX_ERROR_INJECTOR_H
+#define CELIX_ERROR_INJECTOR_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <stddef.h>
+
+#define CELIX_EI_GET_CALLER(addr, level)        \
+do {                                            \
+    Dl_info dlinfo;                             \
+    void *dlret;                                \
+    dlret = __builtin_return_address((level));  \
+    assert(dladdr(dlret, &dlinfo));             \

Review Comment:
   `-rdynamic` is used to make `dladdr` work.  This tech works for release and debug build. 
   However, there is a limitation, if the caller is a static function (not visible outside the current translation unit), it will not work, in which case I will specify `CELIX_EI_UNKNOWN_CALLER` and rely solely on `ordinal`.



-- 
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 merged pull request #473: Add basic error injector mechanism.

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng merged PR #473:
URL: https://github.com/apache/celix/pull/473


-- 
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 #473: Add basic error injector mechanism.

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1084872160


##########
misc/error_injector/celix_properties/celix_properties_ei.cpp:
##########
@@ -0,0 +1,29 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#include <celix_properties_ei.h>

Review Comment:
   Oops! I should have followed Celix's existing convention, and it will be corrected.
   
   In John Lakos's Large Scale C++, it is suggested that angle brackets instead of double quotes should be used exclusively in include directives. I follow his methods in my day-time job. However, existing convention should always take priority.
   
   For more on this: https://learning.oreilly.com/library/view/large-scale-c-volume/9780133927573/ch01.xhtml#ch01lev1sec5



-- 
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 #473: Add basic error injector mechanism.

Posted by GitBox <gi...@apache.org>.
PengZheng commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1071859437


##########
misc/error_injector/error_injector.h:
##########
@@ -0,0 +1,86 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#ifndef CELIX_ERROR_INJECTOR_H
+#define CELIX_ERROR_INJECTOR_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <stddef.h>
+
+#define CELIX_EI_GET_CALLER(addr, level)        \
+do {                                            \
+    Dl_info dlinfo;                             \
+    void *dlret;                                \
+    dlret = __builtin_return_address((level));  \
+    assert(dladdr(dlret, &dlinfo));             \
+    (addr) = dlinfo.dli_saddr;                  \
+} while(0)
+
+#define CELIX_EI_UNKNOWN_CALLER ((void *)-1)
+
+#define CELIX_EI_DECLARE(name, ret_type) \
+void celix_ei_expect_##name(void *caller, unsigned int level, ret_type ret, size_t ordinal=1)
+
+#define CELIX_EI_DEFINE(name, ret_type)                                                     \
+static void *name ## _caller;                                                               \
+static unsigned int name ## _caller_level;                                                  \
+static ret_type name ## _ret;                                                               \
+static size_t name ## _ordinal;                                                             \
+                                                                                            \
+void celix_ei_expect_##name(void *caller, unsigned int level, ret_type ret, size_t ordinal) \
+{                                                                                           \
+    name ## _caller = (caller);                                                             \
+    name ## _caller_level = (level);                                                        \
+    name ## _ret = (ret);                                                                   \
+    name ## _ordinal = (ordinal);                                                           \
+}
+
+
+#define CELIX_EI_IMPL0(name)                                                                             \

Review Comment:
   This is jest one error pattern, in which zero return value is treated as an error. `IMPL0` really means error on zero return value.
   Macros for other patterns (indeed, there are plenty of them) will be added when needed.



##########
misc/error_injector/error_injector.h:
##########
@@ -0,0 +1,86 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#ifndef CELIX_ERROR_INJECTOR_H
+#define CELIX_ERROR_INJECTOR_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <stddef.h>
+
+#define CELIX_EI_GET_CALLER(addr, level)        \
+do {                                            \
+    Dl_info dlinfo;                             \
+    void *dlret;                                \
+    dlret = __builtin_return_address((level));  \
+    assert(dladdr(dlret, &dlinfo));             \
+    (addr) = dlinfo.dli_saddr;                  \
+} while(0)
+
+#define CELIX_EI_UNKNOWN_CALLER ((void *)-1)
+
+#define CELIX_EI_DECLARE(name, ret_type) \
+void celix_ei_expect_##name(void *caller, unsigned int level, ret_type ret, size_t ordinal=1)
+
+#define CELIX_EI_DEFINE(name, ret_type)                                                     \
+static void *name ## _caller;                                                               \
+static unsigned int name ## _caller_level;                                                  \
+static ret_type name ## _ret;                                                               \
+static size_t name ## _ordinal;                                                             \
+                                                                                            \
+void celix_ei_expect_##name(void *caller, unsigned int level, ret_type ret, size_t ordinal) \
+{                                                                                           \
+    name ## _caller = (caller);                                                             \
+    name ## _caller_level = (level);                                                        \
+    name ## _ret = (ret);                                                                   \
+    name ## _ordinal = (ordinal);                                                           \
+}
+
+
+#define CELIX_EI_IMPL0(name)                                                                             \

Review Comment:
   This is jest one error pattern, in which zero return value is treated as an error.
   Macros for other patterns (indeed, there are plenty of them) will be added when needed.



##########
misc/error_injector/error_injector.h:
##########
@@ -0,0 +1,86 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#ifndef CELIX_ERROR_INJECTOR_H
+#define CELIX_ERROR_INJECTOR_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <stddef.h>
+
+#define CELIX_EI_GET_CALLER(addr, level)        \
+do {                                            \
+    Dl_info dlinfo;                             \
+    void *dlret;                                \
+    dlret = __builtin_return_address((level));  \
+    assert(dladdr(dlret, &dlinfo));             \
+    (addr) = dlinfo.dli_saddr;                  \
+} while(0)
+
+#define CELIX_EI_UNKNOWN_CALLER ((void *)-1)
+
+#define CELIX_EI_DECLARE(name, ret_type) \
+void celix_ei_expect_##name(void *caller, unsigned int level, ret_type ret, size_t ordinal=1)
+
+#define CELIX_EI_DEFINE(name, ret_type)                                                     \
+static void *name ## _caller;                                                               \
+static unsigned int name ## _caller_level;                                                  \
+static ret_type name ## _ret;                                                               \
+static size_t name ## _ordinal;                                                             \
+                                                                                            \
+void celix_ei_expect_##name(void *caller, unsigned int level, ret_type ret, size_t ordinal) \
+{                                                                                           \
+    name ## _caller = (caller);                                                             \
+    name ## _caller_level = (level);                                                        \
+    name ## _ret = (ret);                                                                   \
+    name ## _ordinal = (ordinal);                                                           \
+}
+
+
+#define CELIX_EI_IMPL0(name)                                                                             \
+do {                                                                                                     \
+    void *addr = CELIX_EI_UNKNOWN_CALLER;                                                                \
+    if(name##_caller) {                                                                                  \
+        if(name ## _caller != CELIX_EI_UNKNOWN_CALLER) {                                                 \
+            /* we can not use CELIX_EI_GET_CALLER(addr, name ## _caller_level) */                        \
+            switch(name ## _caller_level) {                                                              \
+            case 0:                                                                                      \
+                CELIX_EI_GET_CALLER(addr, 0);                                                            \

Review Comment:
   IIRC, constant is required here.



##########
misc/error_injector/error_injector.h:
##########
@@ -0,0 +1,86 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#ifndef CELIX_ERROR_INJECTOR_H
+#define CELIX_ERROR_INJECTOR_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <stddef.h>
+
+#define CELIX_EI_GET_CALLER(addr, level)        \
+do {                                            \
+    Dl_info dlinfo;                             \
+    void *dlret;                                \
+    dlret = __builtin_return_address((level));  \
+    assert(dladdr(dlret, &dlinfo));             \

Review Comment:
   `-rdynamic` is used to make `dladdr` work.  This tech works for release and debug build. 
   However, there is a limitation, if the caller is a static function (not visible outside the current translation unit), it will not work. 



-- 
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 #473: Add basic error injector mechanism.

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1092718467


##########
libs/error_injector/celix_properties/CMakeLists.txt:
##########
@@ -0,0 +1,24 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+add_library(properties_ei STATIC src/celix_properties_ei.cc)

Review Comment:
   > Is there a reason why properties_ei and malloc_ei cannot be part of the error_injector library?
   
   Yes, it's intentionally made fine-grained (i.e. per API). Suppose that we want to inject errors into a optional 3rd party dependency (e.g. zmq), the corresponding injector will need zmq's header file. By such fine-grained design, we can opt-out the injector as easily as zmq.
   
   > I understand that the current error_injector (interface) library provides the mechanisme to create error injection for functions, but I think we if we use error injection a single linkage setup is a bit user friendly.
   
   Indeed, it brings some inconvenience. Meanwhile it gives a high level overview of which APIs are controlled by the current testing environment.



-- 
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 #473: Add basic error injector mechanism.

Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1091977202


##########
libs/error_injector/README.md:
##########
@@ -0,0 +1,81 @@
+---
+title: Error Injector
+---
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+   
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+# Celix Error Injector

Review Comment:
   :+1: for updating the documentation.
   
   Maybe a list can be added for which functions error injection is already prepared (`malloc` and `celix_properties_create`)?



-- 
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 #473: Add basic error injector mechanism.

Posted by GitBox <gi...@apache.org>.
PengZheng commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1071859437


##########
misc/error_injector/error_injector.h:
##########
@@ -0,0 +1,86 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#ifndef CELIX_ERROR_INJECTOR_H
+#define CELIX_ERROR_INJECTOR_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <stddef.h>
+
+#define CELIX_EI_GET_CALLER(addr, level)        \
+do {                                            \
+    Dl_info dlinfo;                             \
+    void *dlret;                                \
+    dlret = __builtin_return_address((level));  \
+    assert(dladdr(dlret, &dlinfo));             \
+    (addr) = dlinfo.dli_saddr;                  \
+} while(0)
+
+#define CELIX_EI_UNKNOWN_CALLER ((void *)-1)
+
+#define CELIX_EI_DECLARE(name, ret_type) \
+void celix_ei_expect_##name(void *caller, unsigned int level, ret_type ret, size_t ordinal=1)
+
+#define CELIX_EI_DEFINE(name, ret_type)                                                     \
+static void *name ## _caller;                                                               \
+static unsigned int name ## _caller_level;                                                  \
+static ret_type name ## _ret;                                                               \
+static size_t name ## _ordinal;                                                             \
+                                                                                            \
+void celix_ei_expect_##name(void *caller, unsigned int level, ret_type ret, size_t ordinal) \
+{                                                                                           \
+    name ## _caller = (caller);                                                             \
+    name ## _caller_level = (level);                                                        \
+    name ## _ret = (ret);                                                                   \
+    name ## _ordinal = (ordinal);                                                           \
+}
+
+
+#define CELIX_EI_IMPL0(name)                                                                             \

Review Comment:
   This is just one error pattern, in which zero return value is treated as an error. `IMPL0` really means error on zero return value.
   Macros for other patterns (indeed, there are plenty of them) will be added when needed.



-- 
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 #473: Add basic error injector mechanism.

Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1084393570


##########
misc/error_injector/CMakeLists.txt:
##########
@@ -0,0 +1,25 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+add_library(error_injector INTERFACE error_injector.h)

Review Comment:
   nitpick: IMO this library can be subdir of the libs directory and also has as name `Celix::error_injector` 



##########
misc/error_injector/error_injector.h:
##########
@@ -0,0 +1,84 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#ifndef CELIX_ERROR_INJECTOR_H
+#define CELIX_ERROR_INJECTOR_H

Review Comment:
   nitpick: please rename to celix_error_injector.h and move it to a include dir.



##########
misc/error_injector/celix_properties/CMakeLists.txt:
##########
@@ -0,0 +1,23 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+add_library(celix_properties_ei STATIC celix_properties_ei.cpp)

Review Comment:
   nitpick: In Celix most resent C++ sources files use the `cc` extension



##########
bundles/pubsub/pubsub_protocol/pubsub_protocol_lib/gtest/CMakeLists.txt:
##########
@@ -17,10 +17,16 @@
 
 add_executable(celix_pswp_common_tests src/PS_WP_common_tests.cc)
 target_include_directories(celix_pswp_common_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../src)
-target_link_libraries(celix_pswp_common_tests PRIVATE celix_pubsub_protocol_lib GTest::gtest Celix::pubsub_spi GTest::gtest_main ${CMAKE_DL_LIBS})
-if (NOT ENABLE_ADDRESS_SANITIZER)
-    target_compile_definitions(celix_pswp_common_tests PRIVATE ENABLE_MALLOC_RETURN_NULL_TESTS)
-endif ()
+target_link_libraries(celix_pswp_common_tests PRIVATE celix_pubsub_protocol_lib GTest::gtest Celix::pubsub_spi GTest::gtest_main)
 
 add_test(NAME celix_pswp_common_tests COMMAND celix_pswp_common_tests)
-setup_target_for_coverage(celix_pswp_common_tests SCAN_DIR ..)
\ No newline at end of file
+setup_target_for_coverage(celix_pswp_common_tests SCAN_DIR ..)
+
+if (LINKER_WRAP_SUPPORTED)

Review Comment:
   Nice. Yeah so only if wrap is supported this is used and this includes the coverage build.



##########
bundles/pubsub/pubsub_protocol/pubsub_protocol_lib/gtest/src/PS_WP_common_ei_tests.cc:
##########
@@ -0,0 +1,233 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+
+
+#include <gtest/gtest.h>
+#include <iostream>
+#include <cstring>
+#include <celix_properties_ei.h>
+#include <malloc_ei.h>
+
+#include "pubsub_wire_protocol_common.h"
+
+class WireProtocolCommonEiTest : public ::testing::Test {
+public:
+    WireProtocolCommonEiTest() = default;
+    ~WireProtocolCommonEiTest() override {
+        celix_ei_expect_realloc(nullptr, 0, nullptr);
+        celix_ei_expect_calloc(nullptr, 0, nullptr);
+        celix_ei_expect_celix_properties_create(nullptr, 0, nullptr);
+    };
+};
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_EncodeMetadataWithNoMemoryLeft) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 0;
+    message.metadata.metadata = celix_properties_create();
+    celix_properties_set(message.metadata.metadata, "key1", "value1");
+
+    //Scenario: No mem with no pre-allocated data
+    //Given (mocked) realloc is forced to return NULL
+    celix_ei_expect_realloc((void *)pubsubProtocol_encodeMetadata, 0, nullptr);
+
+    //When I try to encode a metadata
+    char *data = nullptr;
+    size_t length = 0;
+    size_t contentLength = 0;
+    auto status = pubsubProtocol_encodeMetadata(&message, &data, &length, &contentLength);
+    //Then I expect a failure
+    EXPECT_NE(status, CELIX_SUCCESS);
+
+    //Scenario: No mem with some pre-allocated data
+    //Given a data set with some space
+    data = (char*)malloc(16);
+    length = 16;
+
+    //And (mocked) realloc is forced to return NULL
+    celix_ei_expect_realloc((void *)pubsubProtocol_encodeMetadata, 0, nullptr);
+
+    //When I try to encode a metadata
+    status = pubsubProtocol_encodeMetadata(&message, &data, &length, &contentLength);
+
+    //Then I expect a failure
+    EXPECT_NE(status, CELIX_SUCCESS);
+
+    free(data);
+    celix_properties_destroy(message.metadata.metadata);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DecodeMetadataWithSingleEntries) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 0;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:value1,"); //note 1 entry
+    auto len = strlen(data);
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 1);
+    auto status = pubsubProtocol_decodeMetadata((void*)data, len, &message);
+
+    EXPECT_EQ(status, CELIX_SUCCESS);
+    EXPECT_EQ(1, celix_properties_size(message.metadata.metadata));
+    EXPECT_STREQ("value1", celix_properties_get(message.metadata.metadata, "key1", "not-found"));
+
+    free(data);
+    celix_properties_destroy(message.metadata.metadata);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DencodeMetadataWithMultipleEntries) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 1;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:value1,4:key2,6:value2,6:key111,8:value111,"); //note 3 entries
+    auto len = strlen(data);
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 3);
+    auto status = pubsubProtocol_decodeMetadata((void*)data, len, &message);
+
+    EXPECT_EQ(status, CELIX_SUCCESS);
+    EXPECT_EQ(3, celix_properties_size(message.metadata.metadata));
+    EXPECT_STREQ("value1", celix_properties_get(message.metadata.metadata, "key1", "not-found"));
+    EXPECT_STREQ("value2", celix_properties_get(message.metadata.metadata, "key2", "not-found"));
+    EXPECT_STREQ("value111", celix_properties_get(message.metadata.metadata, "key111", "not-found"));
+
+    free(data);
+    celix_properties_destroy(message.metadata.metadata);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DencodeMetadataWithExtraEntries) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 1;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:value1,4:key2,6:value2,6:key111,8:value111,"); //note 3 entries
+    auto len = strlen(data);
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 2);
+    auto status = pubsubProtocol_decodeMetadata((void*)data, len, &message);
+    // the 3rd entry should be ignored
+    EXPECT_EQ(status, CELIX_SUCCESS);
+    EXPECT_EQ(2, celix_properties_size(message.metadata.metadata));
+    EXPECT_STREQ("value1", celix_properties_get(message.metadata.metadata, "key1", "not-found"));
+    EXPECT_STREQ("value2", celix_properties_get(message.metadata.metadata, "key2", "not-found"));
+    EXPECT_STREQ("not-found", celix_properties_get(message.metadata.metadata, "key111", "not-found"));
+
+    free(data);
+    celix_properties_destroy(message.metadata.metadata);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DencodeMetadataMissingEntries) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 1;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:value1,4:key2,6:value2,6:key111,8:value111,"); //note 3 entries
+    auto len = strlen(data);
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 4);
+    auto status = pubsubProtocol_decodeMetadata((void*)data, len, &message);
+
+    EXPECT_EQ(status, CELIX_INVALID_SYNTAX);
+    EXPECT_EQ(nullptr, message.metadata.metadata);
+
+    free(data);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DecodeMetadataWithSingleEntryWithIncompleteValue) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 0;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:val,"); //note 1 entry with short value
+    auto len = strlen(data);
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 1);
+    auto status = pubsubProtocol_decodeMetadata((void*)data, len, &message);
+
+    EXPECT_EQ(status, CELIX_INVALID_SYNTAX);
+    EXPECT_EQ(nullptr, message.metadata.metadata);
+
+    free(data);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DecodeMetadataTooShort) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 0;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:value1,"); //note 1 entry
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 1);
+    auto status = pubsubProtocol_decodeMetadata((void*)data, 3, &message); // not enough data for `nOfElements`
+
+    EXPECT_EQ(status, CELIX_INVALID_SYNTAX);
+    EXPECT_EQ(nullptr, message.metadata.metadata);
+
+    free(data);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_DecodeEmptyMetadata) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 0;
+    message.metadata.metadata = nullptr;
+
+    uint32_t data = 0;
+    auto status = pubsubProtocol_decodeMetadata((void*)&data, 4, &message);
+
+    EXPECT_EQ(status, CELIX_SUCCESS);
+    EXPECT_EQ(nullptr, message.metadata.metadata);
+
+    // incorrect `nOfElements`
+    data = 4;
+    status = pubsubProtocol_decodeMetadata((void*)&data, 4, &message);
+
+    EXPECT_EQ(status, CELIX_INVALID_SYNTAX);
+    EXPECT_EQ(nullptr, message.metadata.metadata);
+}
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_NotEnoughMemoryForMultipleEntries) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 1;
+    message.metadata.metadata = nullptr;
+
+    char* data = strdup("ABCD4:key1,6:value1,4:key2,6:value2,6:key111,8:value111,"); //note 3 entries
+    auto len = strlen(data);
+    pubsubProtocol_writeInt((unsigned char*)data, 0, message.header.convertEndianess, 3);
+    for (int i = 0; i < 6; ++i) {
+        celix_ei_expect_calloc((void *)pubsubProtocol_decodeMetadata, 0, nullptr, i+1);

Review Comment:
   Just to check.
   
   So the `i+1` call to realloc from pubsubProtocol_encodeMetadata (0 level deep) should fail and return a nullptr?



##########
misc/error_injector/celix_properties/celix_properties_ei.cpp:
##########
@@ -0,0 +1,29 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#include <celix_properties_ei.h>

Review Comment:
   nitpick: `#include "celix_properties_ei.h"`



##########
misc/error_injector/error_injector.h:
##########
@@ -0,0 +1,84 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#ifndef CELIX_ERROR_INJECTOR_H
+#define CELIX_ERROR_INJECTOR_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <stddef.h>
+

Review Comment:
   Could this macro get some documentation. Something like:
   
   ```
   /**
    * @brief Get the caller library address based on the return address of level frame up t he call stack.
    */
   #define CELIX_EI_GET_CALLER(addr, level)                                   \
   do {                                                                       \
       Dl_info dlinfo;                                                        \
       dladdr(__builtin_return_address(level), &dlinfo);                     \
       (addr) = dlinfo.dli_saddr;                                             \
   } while(0)
   ```



##########
misc/error_injector/malloc/malloc_ei.cpp:
##########
@@ -0,0 +1,43 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#include <malloc_ei.h>
+
+extern "C" {
+void *__real_malloc(size_t);

Review Comment:
   Just to check. the symbol `__real_malloc` which pont to `malloc` due to ld `--wrap`?
   And the any usage of `malloc` will result in symbol `__warp_malloc` due to ld `--wrap`?



##########
bundles/pubsub/pubsub_protocol/pubsub_protocol_lib/gtest/src/PS_WP_common_ei_tests.cc:
##########
@@ -0,0 +1,233 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+
+
+#include <gtest/gtest.h>
+#include <iostream>
+#include <cstring>
+#include <celix_properties_ei.h>
+#include <malloc_ei.h>
+
+#include "pubsub_wire_protocol_common.h"
+
+class WireProtocolCommonEiTest : public ::testing::Test {
+public:
+    WireProtocolCommonEiTest() = default;
+    ~WireProtocolCommonEiTest() override {
+        celix_ei_expect_realloc(nullptr, 0, nullptr);
+        celix_ei_expect_calloc(nullptr, 0, nullptr);
+        celix_ei_expect_celix_properties_create(nullptr, 0, nullptr);
+    };
+};
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_EncodeMetadataWithNoMemoryLeft) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 0;
+    message.metadata.metadata = celix_properties_create();
+    celix_properties_set(message.metadata.metadata, "key1", "value1");
+
+    //Scenario: No mem with no pre-allocated data
+    //Given (mocked) realloc is forced to return NULL
+    celix_ei_expect_realloc((void *)pubsubProtocol_encodeMetadata, 0, nullptr);

Review Comment:
   Just to check. 
   
   So the first call to realloc from pubsubProtocol_encodeMetadata (0 level deep) should fail and return a nullptr?



##########
misc/error_injector/error_injector.h:
##########
@@ -0,0 +1,84 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#ifndef CELIX_ERROR_INJECTOR_H
+#define CELIX_ERROR_INJECTOR_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <stddef.h>
+

Review Comment:
   Or maybe a small readme how to use this (declare, define, etc)



-- 
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 #473: Add basic error injector mechanism.

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1084796728


##########
misc/error_injector/malloc/malloc_ei.cpp:
##########
@@ -0,0 +1,43 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+ */
+
+#include <malloc_ei.h>
+
+extern "C" {
+void *__real_malloc(size_t);

Review Comment:
   Yes, that's the way it works.



-- 
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 pull request #473: Add basic error injector mechanism.

Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on PR #473:
URL: https://github.com/apache/celix/pull/473#issuecomment-1400774103

   > 
   
   IMO support for mac is needed. The main reason for error inject is to ensure that the error handle has coverage and its logic is tested. I think this can be done using linux. Of course we will miss some corner cases, but I do not see a big issue.


-- 
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 #473: Add basic error injector mechanism.

Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1084823103


##########
bundles/pubsub/pubsub_protocol/pubsub_protocol_lib/gtest/src/PS_WP_common_ei_tests.cc:
##########
@@ -0,0 +1,233 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+
+
+#include <gtest/gtest.h>
+#include <iostream>
+#include <cstring>
+#include <celix_properties_ei.h>
+#include <malloc_ei.h>
+
+#include "pubsub_wire_protocol_common.h"
+
+class WireProtocolCommonEiTest : public ::testing::Test {
+public:
+    WireProtocolCommonEiTest() = default;
+    ~WireProtocolCommonEiTest() override {
+        celix_ei_expect_realloc(nullptr, 0, nullptr);
+        celix_ei_expect_calloc(nullptr, 0, nullptr);
+        celix_ei_expect_celix_properties_create(nullptr, 0, nullptr);
+    };
+};
+
+TEST_F(WireProtocolCommonEiTest, WireProtocolCommonTest_EncodeMetadataWithNoMemoryLeft) {
+    pubsub_protocol_message_t message;
+    message.header.convertEndianess = 0;
+    message.metadata.metadata = celix_properties_create();
+    celix_properties_set(message.metadata.metadata, "key1", "value1");
+
+    //Scenario: No mem with no pre-allocated data
+    //Given (mocked) realloc is forced to return NULL
+    celix_ei_expect_realloc((void *)pubsubProtocol_encodeMetadata, 0, nullptr);

Review Comment:
   Yes.



-- 
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 #473: Add basic error injector mechanism.

Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on code in PR #473:
URL: https://github.com/apache/celix/pull/473#discussion_r1091981639


##########
libs/error_injector/celix_properties/CMakeLists.txt:
##########
@@ -0,0 +1,24 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+add_library(properties_ei STATIC src/celix_properties_ei.cc)

Review Comment:
   Is there a reason why properties_ei and malloc_ei cannot be part of the error_injector library?
   
   I understand that the current error_injector (interface) library provides the mechanisme to create error injection for functions, but I think we if we use error injection a single linkage setup is a bit user friendly. 
   



-- 
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 pull request #473: Add basic error injector mechanism.

Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on PR #473:
URL: https://github.com/apache/celix/pull/473#issuecomment-1410419030

   Overall LGTM, I added 2 small but not critical remarks


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