You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/03/02 11:49:39 UTC
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1020: MINIFICPP-1352 Enable -Wall and -Wextra and resolve related warnings
lordgamez commented on a change in pull request #1020:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1020#discussion_r585417128
##########
File path: extensions/coap/nanofi/coap_functions.c
##########
@@ -115,6 +121,7 @@ int create_endpoint_context(coap_context_t **ctx, const char *node, const char *
}
struct coap_pdu_t *create_request(struct coap_context_t *ctx, struct coap_session_t *session, coap_optlist_t **optlist, unsigned char code, coap_str_const_t *ptr) {
+ UNUSED(ctx);
Review comment:
Unfortunately this part is C code, and C99 standard does not allow parameters without names, otherwise you get a `parameter name omitted` error:
[6.9.1.5] If the declarator includes a parameter type list, the declaration of each parameter shall include an identifier, except for the special case of a parameter list consisting of a single parameter of type void, in which case there shall not be an identifier. No declaration list shall follow.
##########
File path: extensions/librdkafka/PublishKafka.cpp
##########
@@ -407,7 +407,7 @@ class ReadCallback : public InputStreamCallback {
return 0;
}
- for (size_t segment_num = 0; read_size_ < flow_size_; ++segment_num) {
+ for (size_t segment_num = 0; gsl::narrow<uint32_t>(read_size_) < flow_size_; ++segment_num) {
Review comment:
I was wondering the same, but I couldn't decide if the value can go below zero, but I suppose it shouldn't. I will change the type.
##########
File path: extensions/librdkafka/rdkafka_utils.h
##########
@@ -47,7 +47,7 @@ struct rd_kafka_conf_deleter {
struct rd_kafka_producer_deleter {
void operator()(rd_kafka_t* ptr) const noexcept {
- rd_kafka_resp_err_t flush_ret = rd_kafka_flush(ptr, 10000 /* ms */); // Matching the wait time of KafkaConnection.cpp
+ // rd_kafka_resp_err_t flush_ret = rd_kafka_flush(ptr, 10000 /* ms */); // Matching the wait time of KafkaConnection.cpp
Review comment:
My mistake, the problem was the unused parameter and I commented it out without realizing that we need the flush call. Thanks for the info.
##########
File path: extensions/pcap/CapturePacket.h
##########
@@ -167,7 +167,7 @@ class CapturePacket : public core::Processor {
REGISTER_RESOURCE(CapturePacket, "CapturePacket captures and writes one or more packets into a PCAP file that will be used as the content of a flow file."
" Configuration options exist to adjust the batching of PCAP files. PCAP batching will place a single PCAP into a flow file. "
- "A regular expression selects network interfaces. Bluetooth network interfaces can be selected through a separate option.")
+ "A regular expression selects network interfaces. Bluetooth network interfaces can be selected through a separate option.");
Review comment:
I changed REGISTER_RESOURCE, removing the semicolon from the macro, so that the semicolon is required now. Previously we sometimes put the semicolon there, sometimes we did not, causing additional semicolon warnings.
##########
File path: extensions/sensors/GetEnvironmentalSensors.h
##########
@@ -75,7 +75,7 @@ class GetEnvironmentalSensors : public SensorBase {
static std::shared_ptr<utils::IdGenerator> id_generator_;
};
-REGISTER_RESOURCE(GetEnvironmentalSensors, "Provides sensor information from known sensors to include humidity and pressure data")
+REGISTER_RESOURCE(GetEnvironmentalSensors, "Provides sensor information from known sensors to include humidity and pressure data");
Review comment:
See the answer above. SMART_ENUM has the semicolon as part of the macro, and it is used consistently.
##########
File path: extensions/script/python/ExecutePythonProcessor.h
##########
@@ -34,6 +34,8 @@
#include "PythonScriptEngine.h"
#include "core/Property.h"
+#pragma GCC visibility push(hidden)
Review comment:
Yes that's the error and those refs are related. There was a similar fix by @szaszm previously in #754, he also mentioned that the fix is due to pybind11 requiring `-fvisibility=hidden`. This solution is now applied in a wider scope by including the classes using PyProcessSession.
##########
File path: CMakeLists.txt
##########
@@ -315,11 +313,28 @@ target_compile_definitions(gsl-lite INTERFACE ${GslDefinitions})
# optional-lite
add_library(optional-lite INTERFACE)
-target_include_directories(optional-lite INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/thirdparty/optional-lite-3.2.0/include")
+target_include_directories(optional-lite SYSTEM INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/thirdparty/optional-lite-3.2.0/include")
# date
include(Date)
+# Setup passthrough args used by third parties
+set(PASSTHROUGH_CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
+set(PASSTHROUGH_CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
+
+# Setup warning flags
+if(MSVC)
+ if(FAIL_ON_WARNINGS)
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /WX")
+ endif()
+else()
Review comment:
Updated in [75e1aa](https://github.com/apache/nifi-minifi-cpp/pull/1020/commits/75e1aa33766b2b43b135ebfd2aac312e796d463a)
##########
File path: extensions/coap/tests/CoapIntegrationBase.h
##########
@@ -45,6 +45,7 @@ class CoapIntegrationBase : public IntegrationBase {
}
void run(const utils::optional<std::string>& test_file_location = {}, const utils::optional<std::string>& bootstrap_file = {}) override {
+ (void)bootstrap_file; // against unused variable warnings
Review comment:
I think I did not realize you can leave the argument unnamed when it has a default value. Updated in [75e1aa](https://github.com/apache/nifi-minifi-cpp/pull/1020/commits/75e1aa33766b2b43b135ebfd2aac312e796d463a)
##########
File path: extensions/librdkafka/PublishKafka.cpp
##########
@@ -407,7 +407,7 @@ class ReadCallback : public InputStreamCallback {
return 0;
}
- for (size_t segment_num = 0; read_size_ < flow_size_; ++segment_num) {
+ for (size_t segment_num = 0; gsl::narrow<uint32_t>(read_size_) < flow_size_; ++segment_num) {
Review comment:
Updated in [75e1aa](https://github.com/apache/nifi-minifi-cpp/pull/1020/commits/75e1aa33766b2b43b135ebfd2aac312e796d463a)
##########
File path: extensions/librdkafka/rdkafka_utils.h
##########
@@ -47,7 +47,7 @@ struct rd_kafka_conf_deleter {
struct rd_kafka_producer_deleter {
void operator()(rd_kafka_t* ptr) const noexcept {
- rd_kafka_resp_err_t flush_ret = rd_kafka_flush(ptr, 10000 /* ms */); // Matching the wait time of KafkaConnection.cpp
+ // rd_kafka_resp_err_t flush_ret = rd_kafka_flush(ptr, 10000 /* ms */); // Matching the wait time of KafkaConnection.cpp
Review comment:
Updated in [75e1aa](https://github.com/apache/nifi-minifi-cpp/pull/1020/commits/75e1aa33766b2b43b135ebfd2aac312e796d463a)
##########
File path: extensions/standard-processors/processors/GenerateFlowFile.cpp
##########
@@ -87,7 +87,7 @@ void generateData(std::vector<char>& data, bool textData = false) {
std::uniform_int_distribution<> distr(0, index_of_last_char);
auto rand = std::bind(distr, eng);
std::generate_n(data.begin(), data.size(), rand);
- std::for_each(data.begin(), data.end(), [](char & c) { c = TEXT_CHARS[c];});
+ std::for_each(data.begin(), data.end(), [](char & c) { c = TEXT_CHARS[static_cast<uint8_t>(c)];});
Review comment:
Sorry I'm not sure what do you mean here. Could you give a code example/suggestion?
##########
File path: extensions/standard-processors/tests/integration/TLSServerSocketSupportedProtocolsTest.cpp
##########
@@ -151,10 +151,12 @@ class SimpleSSLTestClient {
gsl::not_null<std::shared_ptr<logging::Logger>> logger_{gsl::make_not_null(logging::LoggerFactory<SimpleSSLTestClient>::getLogger())};
static SocketDescriptor openConnection(const char *host_name, const char *port, logging::Logger& logger) {
- struct addrinfo hints = {0}, *addrs;
+ struct addrinfo hints;
+ hints.ai_flags = 0;
Review comment:
You are right it wasn't the same behavior and it failed on VS2019 build. I fixed it in [a89e77](https://github.com/apache/nifi-minifi-cpp/pull/1020/commits/a89e77484a6ea3651a2952a6a9236ea0e7027e17). The warning I got from this is that it did not initialize all members with the `{0}` initialization.
----------------------------------------------------------------
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:
users@infra.apache.org