lordgamez commented on code in PR #2174:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2174#discussion_r3279688886
##########
extensions/mqtt/tests/ConsumeMQTTTests.cpp:
##########
@@ -94,6 +96,18 @@ class TestConsumeMQTTProcessor : public
minifi::processors::ConsumeMQTT {
REGISTER_RESOURCE(TestConsumeMQTTProcessor, Processor);
+static std::unique_ptr<MQTTAsync_message,
TestConsumeMQTTProcessor::MQTTMessageDeleter>
+makeMallocMqttMessage(const std::string& payload, int struct_version = 0, int
qos = 0, int retained = 0, int dup = 0, int msgid = 1) {
+ auto* raw_msg =
static_cast<MQTTAsync_message*>(std::malloc(sizeof(MQTTAsync_message))); //
NOLINT(cppcoreguidelines-owning-memory,cppcoreguidelines-no-malloc)
+ auto* payload_copy = static_cast<char*>(std::malloc(payload.size() + 1));
// NOLINT(cppcoreguidelines-owning-memory,cppcoreguidelines-no-malloc)
+ std::memcpy(payload_copy, payload.data(), payload.size());
+ payload_copy[payload.size()] = '\0';
+ *raw_msg = MQTTAsync_message{.struct_id = {'M', 'Q', 'T', 'M'},
.struct_version = struct_version,
+ .payloadlen = gsl::narrow<int>(payload.size()),
.payload = payload_copy,
+ .qos = qos, .retained = retained, .dup = dup,
.msgid = msgid, .properties = {}};
+ return std::unique_ptr<MQTTAsync_message,
TestConsumeMQTTProcessor::MQTTMessageDeleter>(raw_msg); //
NOLINT(clang-analyzer-unix.Malloc)
+}
+
Review Comment:
This was a bug that previously we were using `new` for the test message
allocation, while the `MQTTMessageDeleter` was using `MQTTAsync_freeMessage`
which uses `free` for deallocation. The problem did not occur before as we
weren't using the `PAHO_HIGH_PERFORMANCE` cmake option, which made the mqtt
library use its own allocator that had a heap tracker. As the heap tracker did
not track our `new` created message it was a noop when calling
`MQTTAsync_freeMessage` and leaked the message object.
With the introduction of `PAHO_HIGH_PERFORMANCE` cmake option it crashed
because it uses `free` directly on the `new` created message. With this change
`malloc` is used to create the message to avoid the issue.
##########
CMakeLists.txt:
##########
@@ -302,7 +301,14 @@ include(GslLite)
# Add necessary definitions based on the value of STRICT_GSL_CHECKS, see
gsl-lite README for more details
list(APPEND GslDefinitions gsl_CONFIG_DEFAULTS_VERSION=1)
-list(APPEND GslDefinitionsNonStrict gsl_CONFIG_CONTRACT_VIOLATION_THROWS
gsl_CONFIG_NARROW_THROWS_ON_TRUNCATION=1)
+list(APPEND GslDefinitions gsl_FEATURE_BYTE=1)
+list(APPEND GslDefinitions gsl_CONFIG_NARROW_THROWS_ON_TRUNCATION=1)
Review Comment:
This was my mistake putting it in the `GslDefinitionsNonStrict` definitions
which is only used in Debug mode, it should be defined in all cases.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
