[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1089: MINIFICPP-1567 enable linter checks in extensions (part 1)

[GitBox <gi...@apache.org>]

szaszm commented on a change in pull request #1089:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1089#discussion_r642561026



##########
File path: extensions/coap/controllerservice/CoapConnector.h
##########
@@ -15,8 +15,12 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#ifndef LIBMINIFI_INCLUDE_CONTROLLERS_COAPCONNECTOR_H_
-#define LIBMINIFI_INCLUDE_CONTROLLERS_COAPCONNECTOR_H_
+#pragma once
+
+#include <memory>
+#include <string>
+#include <unordered_map>

Review comment:
       `<unordered_map>` is unused here, but `<mutex>` and `<atomic>` are missing.

##########
File path: extensions/windows-event-log/Bookmark.cpp
##########
@@ -19,6 +19,8 @@
 #include "Bookmark.h"
 
 #include <direct.h>
+#include <vector>
+#include <unordered_map>

Review comment:
       missing `<fstream>`

##########
File path: extensions/windows-event-log/CollectorInitiatedSubscription.cpp
##########
@@ -19,16 +19,17 @@
  */
 
 #include "CollectorInitiatedSubscription.h"
+#include <stdio.h>

Review comment:
       `<stdio.h>` and `<iostream>` are unused unless I missed something

##########
File path: extensions/coap/controllerservice/CoapMessaging.h
##########
@@ -15,15 +15,16 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#ifndef EXTENSIONS_COAP_CONTROLLERSERVICE_COAPMESSAGING_H_
-#define EXTENSIONS_COAP_CONTROLLERSERVICE_COAPMESSAGING_H_
+#pragma once
+
+#include <unordered_map>
+#include <memory>
+#include <utility>

Review comment:
       `<memory>` is unused but `<mutex>` is missing

##########
File path: extensions/windows-event-log/ConsumeWindowsEventLog.cpp
##########
@@ -127,8 +129,8 @@ core::Property ConsumeWindowsEventLog::EventHeaderDelimiter(
 core::Property ConsumeWindowsEventLog::EventHeader(
   core::PropertyBuilder::createProperty("Event Header")->
   isRequired(false)->
-  withDefaultValue("LOG_NAME=Log Name, SOURCE = Source, TIME_CREATED = Date,EVENT_RECORDID=Record ID,EVENTID = Event ID,TASK_CATEGORY = Task Category,LEVEL = Level,KEYWORDS = Keywords,USER = User,COMPUTER = Computer, EVENT_TYPE = EventType")->
-  withDescription("Comma seperated list of key/value pairs with the following keys LOG_NAME, SOURCE, TIME_CREATED,EVENT_RECORDID,EVENTID,TASK_CATEGORY,LEVEL,KEYWORDS,USER,COMPUTER, and EVENT_TYPE. Eliminating fields will remove them from the header.")->
+  withDefaultValue("LOG_NAME=Log Name, SOURCE = Source, TIME_CREATED = Date,EVENT_RECORDID=Record ID,EVENTID = Event ID,TASK_CATEGORY = Task Category,LEVEL = Level,KEYWORDS = Keywords,USER = User,COMPUTER = Computer, EVENT_TYPE = EventType")->  // NOLINT linelength
+  withDescription("Comma seperated list of key/value pairs with the following keys LOG_NAME, SOURCE, TIME_CREATED,EVENT_RECORDID,EVENTID,TASK_CATEGORY,LEVEL,KEYWORDS,USER,COMPUTER, and EVENT_TYPE. Eliminating fields will remove them from the header.")-> // NOLINT linelength

Review comment:
       I vote against the split, because it's just a single line literal, but I would move the `->` operators to the beginning of the lines.
   
   See also https://google.github.io/styleguide/cppguide.html#Line_Length (unclear for this case IMO).

##########
File path: extensions/windows-event-log/TailEventLog.h
##########
@@ -120,10 +121,10 @@ class TailEventLog : public core::Processor {
       (LPTSTR)&lpMsg,
       0, NULL);
 
-    logger_->log_debug("Error %d: %s\n", (int)error_id, (char *)lpMsg);
+    logger_->log_debug("Error %d: %s\n", static_cast<int>(error_id), reinterpret_cast<char *>(lpMsg));

Review comment:
       There's a memory leak here. `lpMsg` should be freed with `LocalFree` according to [MSDN](https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessage#FORMAT_MESSAGE_ALLOCATE_BUFFER).

##########
File path: extensions/coap/protocols/CoapC2Protocol.h
##########
@@ -15,8 +15,10 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#ifndef EXTENSIONS_COAPPROTOCOL_H_
-#define EXTENSIONS_COAPPROTOCOL_H_
+#pragma once
+
+#include <stdio.h>
+#include <string.h>

Review comment:
       `<stdio.h>` seems to be unused
   `<string.h>` could be replaced with the c++-style version `<cstring>`
   `<stdexcept>`, `<mutex>` are missing

##########
File path: extensions/windows-event-log/wel/XMLString.h
##########
@@ -20,18 +20,22 @@
 
 #pragma once
 
-#include "core/Core.h"
-#include "FlowFileRecord.h"
-#include "concurrentqueue.h"
-#include "core/Processor.h"
-#include "core/ProcessSession.h"
-#include <pugixml.hpp>
+#include <Windows.h>
 #include <winevt.h>
+
+#include <codecvt>

Review comment:
       `<codecvt>` seems to be unused.

##########
File path: extensions/coap/nanofi/coap_functions.h
##########
@@ -25,12 +25,13 @@ extern "C" {
 
 typedef unsigned char method_t;
 
+#include <stdio.h>
+#include <string.h>

Review comment:
       I think both of these are unused.

##########
File path: extensions/windows-event-log/tests/CWELTestUtils.h
##########
@@ -17,6 +17,10 @@
 
 #pragma once
 
+#include <utility>
+#include <memory>
+#include <string>

Review comment:
       Missing `<fstream>` (`std::ifstream` at line 80) and `<iterator>` (`std::istreambuf_iterator` at line 81).

##########
File path: extensions/windows-event-log/TailEventLog.cpp
##########
@@ -19,14 +19,16 @@
  */
 
 #include "TailEventLog.h"
+#include <stdio.h>

Review comment:
       I think this, `<queue>` , `<sstream>` and `<iostream>` are unused

##########
File path: extensions/coap/server/CoapServer.cpp
##########
@@ -17,6 +17,7 @@
  */
 #include "CoapServer.h"
 #include <coap2/coap.h>
+#include <map>

Review comment:
       also missing `<functional>` for `std::function`

##########
File path: extensions/coap/server/CoapServer.h
##########
@@ -16,16 +16,20 @@
  * limitations under the License.
  */
 
-#ifndef EXTENSIONS_COAP_SERVER_COAPSERVER_H_
-#define EXTENSIONS_COAP_SERVER_COAPSERVER_H_
+#pragma once
 
-#include "core/Connectable.h"
-#include "coap_server.h"
-#include "coap_message.h"
 #include <coap2/coap.h>
 #include <functional>
 #include <thread>
 #include <future>
+#include <map>
+#include <memory>
+#include <string>
+#include <utility>

Review comment:
       Missing `<atomic>`

##########
File path: extensions/windows-event-log/wel/UnicodeConversion.h
##########
@@ -20,27 +20,27 @@
 
 #pragma once
 
-#include <string>
-
 #include <atlbase.h>
 #include <atlconv.h>
 
+#include <string>
+
 namespace org {
-  namespace apache {
-    namespace nifi {
-      namespace minifi {
-        namespace wel {
-          inline std::string to_string(const wchar_t* pChar) {
-            ATL::CW2A aString(pChar, CP_UTF8);
-            return std::string(aString);
-          }
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace wel {
+  inline std::string to_string(const wchar_t* pChar) {
+    ATL::CW2A aString(pChar, CP_UTF8);
+    return std::string(aString);
+  }
 
-          inline std::wstring to_wstring(const char* pChar) {
-            ATL::CA2W wString(pChar, CP_UTF8);
-            return std::wstring(wString);
-          }
-        } /* namespace wel */
-      } /* namespace minifi */
-    } /* namespace nifi */
-  } /* namespace apache */
+  inline std::wstring to_wstring(const char* pChar) {
+    ATL::CA2W wString(pChar, CP_UTF8);
+    return std::wstring(wString);
+  }

Review comment:
       These should still not be indented. Namespace contents are not indented.

##########
File path: extensions/windows-event-log/wel/MetadataWalker.cpp
##########
@@ -17,9 +17,15 @@
  */
 
 #include <windows.h>
+#include <strsafe.h>
+
+#include <map>
+#include <string>
+#include <utility>
+#include <vector>

Review comment:
       Missing: `<functional>`, `<codecvt>`, `<regex>`, 

##########
File path: extensions/windows-event-log/wel/JSONUtils.h
##########
@@ -20,9 +20,10 @@
 #undef RAPIDJSON_ASSERT
 #define RAPIDJSON_ASSERT(x) if (!(x)) throw std::logic_error("rapidjson exception");  // NOLINT
 
-#include <pugixml.hpp>
-
 #include <stdexcept>  // for std::logic_error
+#include <string>

Review comment:
       `<stdexcept>` can go to the implementation file, it's not used in the header.




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