szaszm commented on code in PR #1434:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1434#discussion_r1052248325
##########
extensions/windows-event-log/wel/MetadataWalker.h:
##########
@@ -95,16 +90,12 @@ class MetadataWalker : public pugi::xml_tree_walker {
const WindowsEventLogMetadata& windows_event_log_metadata_;
std::string log_name_;
- std::optional<utils::Regex> regex_;
+ const std::optional<utils::Regex>& regex_;
Review Comment:
I feel like a nullable raw pointer to regex is similar enough that it
probably fulfills the requirements, but it's a simpler representation, so I
would rather go with that if it works. But only if this class doesn't need to
react to concurrent changes of the regex data member.
##########
libminifi/src/utils/OsUtils.cpp:
##########
@@ -284,6 +284,22 @@ int64_t OsUtils::getTotalPagingFileSize() {
DWORDLONG total_paging_file_size = memory_info.ullTotalPageFile;
return total_paging_file_size;
}
+
+std::string OsUtils::getWindowsErrorAsString(DWORD error_code) {
+ if (error_code == 0)
+ return "";
+
+ LPSTR message_buffer = nullptr;
+
+ size_t size = FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER |
FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
+ nullptr, error_code, MAKELANGID(LANG_NEUTRAL,
SUBLANG_DEFAULT),
+ (LPSTR)&message_buffer, 0, nullptr);
+
+ std::string message(message_buffer, size);
+
+ LocalFree(message_buffer);
+ return message;
+}
Review Comment:
It's much simpler and safer to use std::error_code and related facilities to
convert windows-specific error codes to std::error_code, then use .message()
(returns a string) instead of FormatMessage. The narrowing conversion from
DWORD to int is the only unsafe step, but all windows error codes fit in both,
so it's fine. The codes go from 0 to 16k.
https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes
```suggestion
std::string OsUtils::getWindowsErrorAsString(DWORD error_code) {
return std::error_code{gsl::narrow<int>(error_code),
std::system_category()}.message();
}
```
##########
extensions/windows-event-log/wel/WindowsEventLog.cpp:
##########
@@ -158,51 +159,59 @@ std::string
WindowsEventLogMetadataImpl::getEventData(EVT_FORMAT_MESSAGE_FLAGS f
return event_data;
}
-std::string WindowsEventLogHandler::getEventMessage(EVT_HANDLE eventHandle)
const {
+nonstd::expected<std::string, DWORD>
WindowsEventLogHandler::getEventMessage(EVT_HANDLE eventHandle) const {
Review Comment:
I suggest using `std::error_code` as the unexpected type.
##########
extensions/windows-event-log/wel/WindowsEventLog.cpp:
##########
@@ -25,64 +25,67 @@
#include "UnicodeConversion.h"
#include "utils/Deleters.h"
#include "utils/gsl.h"
+#include "UniqueEvtHandle.h"
namespace org::apache::nifi::minifi::wel {
+namespace {
+std::string GetEventTimestampStr(uint64_t event_timestamp) {
+ SYSTEMTIME st;
+ FILETIME ft;
+
+ ft.dwHighDateTime = (DWORD)((event_timestamp >> 32) & 0xFF'FF'FF'FF);
+ ft.dwLowDateTime = (DWORD)(event_timestamp & 0xFF'FF'FF'FF);
+
+ FileTimeToSystemTime(&ft, &st);
+ std::stringstream datestr;
+
+ std::string period = "AM";
+ auto hour = st.wHour;
+ if (hour >= 12 && hour < 24)
+ period = "PM";
+ if (hour > 12)
+ hour -= 12;
+ if (hour == 0)
+ hour = 12;
+ datestr << st.wMonth << "/" << st.wDay << "/" << st.wYear << " " <<
std::setfill('0') << std::setw(2) << hour << ":" << std::setfill('0')
+ << std::setw(2) << st.wMinute << ":" << std::setfill('0') <<
std::setw(2) << st.wSecond << " " << period;
+ return datestr.str();
+}
+} // namespace
Review Comment:
Is there any way to convert the timestamp using standard date/time
formatting facilities, or date.h? Because these are complex problems with
existing solutions, and I can't confidently say that this covers all edge
cases. (endianness, timezones, DST and so on)
--
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]
