fgerlits commented on a change in pull request #1040:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1040#discussion_r605089947
##########
File path: extensions/standard-processors/processors/GetTCP.cpp
##########
@@ -130,7 +130,7 @@ void GetTCP::onSchedule(const
std::shared_ptr<core::ProcessContext> &context, co
}
if (context->getProperty(StayConnected.getName(), value)) {
- utils::StringUtils::StringToBool(value, stay_connected_);
+ stay_connected_ = utils::StringUtils::toBool(value).value_or(false);
Review comment:
I would make the default true, because we set `stay_connected_` to true
in the `else` branch.
##########
File path: extensions/mqtt/processors/AbstractMQTTProcessor.cpp
##########
@@ -146,7 +148,7 @@ void AbstractMQTTProcessor::onSchedule(const
std::shared_ptr<core::ProcessContex
MQTTClient_create(&client_, uri_.c_str(), clientID_.c_str(),
MQTTCLIENT_PERSISTENCE_NONE, NULL);
}
if (client_) {
- MQTTClient_setCallbacks(client_, (void *) this, connectionLost,
msgReceived, msgDelivered);
+ MQTTClient_setCallbacks(client_, reinterpret_cast<void *> (this),
connectionLost, msgReceived, msgDelivered);
Review comment:
I don't think a cast is needed at all; `void*` is "pointer to anything"
so `AbstractMQTTProcessor*` should be implicitly convertable to `void*`.
##########
File path: libminifi/include/utils/StringUtils.h
##########
@@ -75,13 +75,12 @@ struct string_traits<wchar_t>{
class StringUtils {
public:
/**
- * Converts a string to a boolean
- * Better handles mixed case.
+ * Checks and converts a string to a boolean
* @param input input string
- * @param output output string.
+ * @returns an optional of a boolean: true if the string is "true" (ignoring
case), false if it is "false" (ignoring case), nullopt for any other value
*/
- static bool StringToBool(std::string input, bool &output);
+ static bool StringToBool(std::string input, bool &output);
Review comment:
this can be deleted now, I think
##########
File path: extensions/standard-processors/processors/HashContent.cpp
##########
@@ -62,11 +63,8 @@ void HashContent::onSchedule(core::ProcessContext *context,
core::ProcessSession
attrKey_ = (context->getProperty(HashAttribute.getName(), value)) ? value :
"Checksum";
algoName_ = (context->getProperty(HashAlgorithm.getName(), value)) ? value :
"SHA256";
- if (context->getProperty(HashAlgorithm.getName(), value)) {
- bool bool_value;
- failOnEmpty_ = utils::StringUtils::StringToBool(value, bool_value) &&
bool_value; // Only true in case of valid true string
- } else {
- failOnEmpty_ = false;
+ if (context->getProperty(FailOnEmpty.getName(), value)) {
+ failOnEmpty_ = utils::StringUtils::toBool(value).value_or(false);
Review comment:
We need to set `failOnEmpty_` to false when `getProperty()` returns
false. You can use either an else branch, or a `getProperty(...) &&
toBool(...).value_or(false)` expression similar to what you used elsewhere.
##########
File path: controller/MiNiFiController.cpp
##########
@@ -130,11 +128,11 @@ int main(int argc, char **argv) {
if ((IsNullOrEmpty(host) && port == -1)) {
std::cout << "MiNiFi Controller is disabled" << std::endl;
exit(0);
- } else
-
+ } else {
if (result.count("noheaders")) {
show_headers = false;
}
+ }
Review comment:
I think the correct fix is to remove the `else` keyword from the
original code.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
