diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..43f8989 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,63 @@ +--- +# Resons why specific warnings have been turned off: +# +# -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling +# This warns about memcpy and wants us to use memcpy_s, which is not available in our gcc setup. +# +# -cppcoreguidelines-pro-type-vararg +# This forbids using functions like printf, snprintf etc. We would like to use those either way. +# +# -misc-no-recursion +# Recursion with functions can be an elegant way of solving recursive problems +# +# These checks have been disabled to keep compatibility with C++14: +# -modernize-concat-nested-namespaces +# -modernize-use-nodiscard +# + +Checks: "-*, + clang-analyzer-*, + -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling, + + bugprone-*, + -bugprone-easily-swappable-parameters, + -bugprone-implicit-widening-of-multiplication-result, + -bugprone-narrowing-conversions, + + cppcoreguidelines-*, + -cppcoreguidelines-avoid-magic-numbers, + -cppcoreguidelines-avoid-non-const-global-variables, + -cppcoreguidelines-macro-usage, + -cppcoreguidelines-narrowing-conversions, + -cppcoreguidelines-non-private-member-variables-in-classes, + -cppcoreguidelines-pro-bounds-array-to-pointer-decay, + -cppcoreguidelines-pro-bounds-pointer-arithmetic, + -cppcoreguidelines-pro-type-vararg, + -cppcoreguidelines-pro-type-reinterpret-cast, + + misc-*, + -misc-non-private-member-variables-in-classes, + -misc-no-recursion, + + modernize-*, + -modernize-pass-by-value, + -modernize-use-trailing-return-type, + -modernize-use-auto, + -modernize-concat-nested-namespaces, + -modernize-return-braced-init-list, + -modernize-use-nodiscard, + -modernize-avoid-bind, + + performance-*, + + readability-*, + -readability-braces-around-statements, + -readability-identifier-length, + -readability-magic-numbers, + -readability-redundant-access-specifiers, + -readability-function-cognitive-complexity, + -readability-else-after-return, +" +WarningsAsErrors: '' +HeaderFilterRegex: '^((?!/thirdparty/|/_deps/).)*$' +FormatStyle: none diff --git a/samples/asio_sender_multicast/src/main.cpp b/samples/asio_sender_multicast/src/main.cpp index 2bf640a..131ed72 100644 --- a/samples/asio_sender_multicast/src/main.cpp +++ b/samples/asio_sender_multicast/src/main.cpp @@ -32,12 +32,12 @@ int main() asio::io_service io_service; - asio::ip::udp::endpoint endpoint(asio::ip::make_address("239.0.0.1"), 14000); - asio::ip::udp::socket upd_socket(io_service, endpoint.protocol()); + const asio::ip::udp::endpoint endpoint(asio::ip::make_address("239.0.0.1"), 14000); + asio::ip::udp::socket upd_socket(io_service, endpoint.protocol()); // set multicast packet TTL { - asio::ip::multicast::hops ttl(2); + const asio::ip::multicast::hops ttl(2); asio::error_code ec; upd_socket.set_option(ttl, ec); if (ec) @@ -49,9 +49,9 @@ int main() // set loopback option { - asio::ip::multicast::enable_loopback loopback(true); + const asio::ip::multicast::enable_loopback loopback(true); asio::error_code ec; - upd_socket.set_option(loopback); + upd_socket.set_option(loopback, ec); if (ec) { std::cerr << "ERROR: Error setting loopback option: " << ec.message() << std::endl; diff --git a/samples/asio_sender_unicast/src/main.cpp b/samples/asio_sender_unicast/src/main.cpp index d4690dd..b89fe26 100644 --- a/samples/asio_sender_unicast/src/main.cpp +++ b/samples/asio_sender_unicast/src/main.cpp @@ -32,8 +32,8 @@ int main() asio::io_service io_service; - asio::ip::udp::endpoint endpoint(asio::ip::make_address("127.0.0.1"), 14000); - asio::ip::udp::socket upd_socket(io_service, endpoint.protocol()); + const asio::ip::udp::endpoint endpoint(asio::ip::make_address("127.0.0.1"), 14000); + asio::ip::udp::socket upd_socket(io_service, endpoint.protocol()); int counter = 0; for(;;) diff --git a/udpcap/include/udpcap/host_address.h b/udpcap/include/udpcap/host_address.h index 7929b50..e6ba5ce 100644 --- a/udpcap/include/udpcap/host_address.h +++ b/udpcap/include/udpcap/host_address.h @@ -59,8 +59,6 @@ namespace Udpcap /** @brief Constructs a HostAddress from a 32bit integer in host byte order. */ UDPCAP_EXPORT HostAddress(const uint32_t address); - UDPCAP_EXPORT ~HostAddress(); - /** @brief Checks if the Host Address is valid. * Invalid HostAddresses are created when providing a wrong IPv4 string, * using the empty default constructor or the HostAddress::Invalid() diff --git a/udpcap/include/udpcap/udpcap_socket.h b/udpcap/include/udpcap/udpcap_socket.h index d4ca671..35eae2d 100644 --- a/udpcap/include/udpcap/udpcap_socket.h +++ b/udpcap/include/udpcap/udpcap_socket.h @@ -76,8 +76,13 @@ namespace Udpcap UDPCAP_EXPORT UdpcapSocket(); UDPCAP_EXPORT ~UdpcapSocket(); - UDPCAP_EXPORT UdpcapSocket(UdpcapSocket const&) = delete; - UDPCAP_EXPORT UdpcapSocket& operator= (UdpcapSocket const&) = delete; + // Copy + UdpcapSocket(UdpcapSocket const&) = delete; + UdpcapSocket& operator= (UdpcapSocket const&) = delete; + + // Move + UDPCAP_EXPORT UdpcapSocket& operator=(UdpcapSocket&&) noexcept; + UDPCAP_EXPORT UdpcapSocket(UdpcapSocket&&) noexcept; /** * @brief Checks whether the socket is valid (i.e. npcap has been intialized successfully) diff --git a/udpcap/src/host_address.cpp b/udpcap/src/host_address.cpp index 7a2c39b..7501164 100644 --- a/udpcap/src/host_address.cpp +++ b/udpcap/src/host_address.cpp @@ -23,6 +23,8 @@ #define NOMINMAX #include +#include + namespace Udpcap { //////////////////////////////// @@ -46,9 +48,6 @@ namespace Udpcap , ipv4_(address) {} - HostAddress::~HostAddress() - {} - bool HostAddress::isValid() const { return valid_; @@ -70,7 +69,7 @@ namespace Udpcap { if (valid_) { - uint32_t lower_byte = (ipv4_ & 0x000000FF); + const uint32_t lower_byte = (ipv4_ & 0x000000FF); return (lower_byte >= 224) && (lower_byte <= 239); } else @@ -83,9 +82,9 @@ namespace Udpcap { if (valid_) { - char buffer[16]; - inet_ntop(AF_INET, (void*)(&ipv4_), buffer, 16); - return std::string(buffer); + std::array buffer{}; + inet_ntop(AF_INET, (void*)(&ipv4_), buffer.data(), 16); + return std::string(buffer.data()); } else { diff --git a/udpcap/src/ip_reassembly.cpp b/udpcap/src/ip_reassembly.cpp index b89a76a..725a5c4 100644 --- a/udpcap/src/ip_reassembly.cpp +++ b/udpcap/src/ip_reassembly.cpp @@ -83,11 +83,11 @@ namespace Udpcap /// Helper functions ///////////////////////////////////////// - std::unique_ptr IpReassembly::getPacketKey(pcpp::Packet* packet) + std::unique_ptr IpReassembly::getPacketKey(const pcpp::Packet* packet) { { pcpp::IPv4Layer* ipv4_layer = packet->getLayerOfType(); - if (ipv4_layer) + if (ipv4_layer != nullptr) { return std::make_unique(_byteswap_ushort(ipv4_layer->getIPv4Header()->ipId), ipv4_layer->getSrcIPv4Address(), ipv4_layer->getDstIPv4Address()); } @@ -95,10 +95,10 @@ namespace Udpcap { pcpp::IPv6Layer* ipv6_layer = packet->getLayerOfType(); - if (ipv6_layer) + if (ipv6_layer != nullptr) { - auto frag_header = ipv6_layer->getExtensionOfType(); - if (frag_header) + auto* frag_header = ipv6_layer->getExtensionOfType(); + if (frag_header != nullptr) return std::make_unique(ntohl(frag_header->getFragHeader()->id), ipv6_layer->getSrcIPv6Address(), ipv6_layer->getDstIPv6Address()); else return nullptr; @@ -149,7 +149,8 @@ namespace Udpcap } else { - timestamp_map_.emplace(packet_key->getHashValue(), std::make_pair(std::move(packet_key), now)); + auto hash_value = packet_key->getHashValue(); + timestamp_map_.emplace(hash_value, std::make_pair(std::move(packet_key), now)); } } diff --git a/udpcap/src/ip_reassembly.h b/udpcap/src/ip_reassembly.h index 5409b9f..33968d6 100644 --- a/udpcap/src/ip_reassembly.h +++ b/udpcap/src/ip_reassembly.h @@ -138,7 +138,7 @@ namespace Udpcap ///////////////////////////////////////// private: - std::unique_ptr getPacketKey(pcpp::Packet* packet); + static std::unique_ptr getPacketKey(const pcpp::Packet* packet); void removeOldPackages(); void removePackageFromTimestampMap(std::unique_ptr packet_key); diff --git a/udpcap/src/npcap_helpers.cpp b/udpcap/src/npcap_helpers.cpp index 7620f4a..83d0ad3 100644 --- a/udpcap/src/npcap_helpers.cpp +++ b/udpcap/src/npcap_helpers.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -39,32 +40,32 @@ namespace Udpcap { namespace // Private Namespace { - static std::mutex npcap_mutex; - static bool is_initialized(false); + std::mutex npcap_mutex; + bool is_initialized(false); - static std::string loopback_device_uuid_string; - static bool loopback_device_name_initialized(false); + std::string loopback_device_uuid_string; + bool loopback_device_name_initialized(false); - static std::string human_readible_error_("Npcap has not been initialized, yet"); + std::string human_readible_error_("Npcap has not been initialized, yet"); bool LoadNpcapDlls() { - _TCHAR npcap_dir[512]; - UINT len; - len = GetSystemDirectory(npcap_dir, 480); - if (!len) { + std::array<_TCHAR, 512> npcap_dir{}; + UINT len(0); + len = GetSystemDirectory(npcap_dir.data(), 480); + if (len == 0) { human_readible_error_ = "Error in GetSystemDirectory"; fprintf(stderr, "Error in GetSystemDirectory: %x", GetLastError()); return false; } - _tcscat_s(npcap_dir, 512, _T("\\Npcap")); - if (SetDllDirectory(npcap_dir) == 0) { + _tcscat_s(npcap_dir.data(), 512, _T("\\Npcap")); + if (SetDllDirectory(npcap_dir.data()) == 0) { human_readible_error_ = "Error in SetDllDirectory"; fprintf(stderr, "Error in SetDllDirectory: %x", GetLastError()); return false; } - if(LoadLibrary("wpcap.dll") == NULL) + if(LoadLibrary("wpcap.dll") == nullptr) return false; return true; @@ -75,12 +76,12 @@ namespace Udpcap nValue = nDefaultValue; DWORD dwBufferSize(sizeof(DWORD)); DWORD nResult(0); - LONG nError = ::RegQueryValueExW(hKey, - strValueName.c_str(), - 0, - NULL, - reinterpret_cast(&nResult), - &dwBufferSize); + const LONG nError = ::RegQueryValueExW(hKey, + strValueName.c_str(), + nullptr, + nullptr, + reinterpret_cast(&nResult), + &dwBufferSize); if (ERROR_SUCCESS == nError) { nValue = nResult; @@ -91,12 +92,12 @@ namespace Udpcap LONG GetBoolRegKey(HKEY hKey, const std::wstring &strValueName, bool &bValue, bool bDefaultValue) { - DWORD nDefValue((bDefaultValue) ? 1 : 0); + const DWORD nDefValue((bDefaultValue) ? 1 : 0); DWORD nResult(nDefValue); - LONG nError = GetDWORDRegKey(hKey, strValueName, nResult, nDefValue); + const LONG nError = GetDWORDRegKey(hKey, strValueName, nResult, nDefValue); if (ERROR_SUCCESS == nError) { - bValue = (nResult != 0) ? true : false; + bValue = (nResult != 0); } return nError; } @@ -105,22 +106,22 @@ namespace Udpcap LONG GetStringRegKey(HKEY hKey, const std::wstring &strValueName, std::wstring &strValue, const std::wstring &strDefaultValue) { strValue = strDefaultValue; - WCHAR szBuffer[512]; + std::array szBuffer{}; DWORD dwBufferSize = sizeof(szBuffer); - ULONG nError; - nError = RegQueryValueExW(hKey, strValueName.c_str(), 0, NULL, (LPBYTE)szBuffer, &dwBufferSize); + ULONG nError(0); + nError = RegQueryValueExW(hKey, strValueName.c_str(), nullptr, nullptr, reinterpret_cast(szBuffer.data()), &dwBufferSize); if (ERROR_SUCCESS == nError) { - strValue = szBuffer; + strValue = szBuffer.data(); } return nError; } bool LoadLoopbackDeviceNameFromRegistry() { - HKEY hkey; - LONG error_code = RegOpenKeyExW(HKEY_LOCAL_MACHINE, L"SYSTEM\\CurrentControlSet\\Services\\npcap\\Parameters", 0, KEY_READ, &hkey); - if (error_code) + HKEY hkey = nullptr; + const LONG error_code = RegOpenKeyExW(HKEY_LOCAL_MACHINE, L"SYSTEM\\CurrentControlSet\\Services\\npcap\\Parameters", 0, KEY_READ, &hkey); + if (error_code != 0) { human_readible_error_ = "NPCAP doesn't seem to be installed. Please download and install Npcap from https://nmap.org/npcap/#download"; std::cerr << "Udpcap ERROR: " << human_readible_error_ << std::endl; @@ -162,12 +163,12 @@ namespace Udpcap RegCloseKey(hkey); std::wstring_convert, wchar_t> converter; - std::string loopback_device_name = converter.to_bytes(loopback_device_name_w); + const std::string loopback_device_name = converter.to_bytes(loopback_device_name_w); // The Registry entry is in the form: \Device\{6DBF8591-55F9-4DEF-A317-54B9563A42E3} // We however only want the UUID: 6DBF8591-55F9-4DEF-A317-54B9563A42E3 - size_t open_bracket_pos = loopback_device_name.find('{'); - size_t closing_bracket_pos = loopback_device_name.find('}', open_bracket_pos); + const size_t open_bracket_pos = loopback_device_name.find('{'); + const size_t closing_bracket_pos = loopback_device_name.find('}', open_bracket_pos); loopback_device_uuid_string = loopback_device_name.substr(open_bracket_pos + 1, closing_bracket_pos - open_bracket_pos - 1); return true; @@ -208,21 +209,21 @@ namespace Udpcap { typedef std::unique_ptr pcap_if_t_uniqueptr; - char errbuf[PCAP_ERRBUF_SIZE]; - pcap_if_t* alldevs_rawptr; - pcap_if_t_uniqueptr alldevs(&alldevs_rawptr, [](pcap_if_t** p) { pcap_freealldevs(*p); }); + std::array errbuf{}; + pcap_if_t* alldevs_rawptr = nullptr; + const pcap_if_t_uniqueptr alldevs(&alldevs_rawptr, [](pcap_if_t** p) { pcap_freealldevs(*p); }); bool loopback_device_found = false; - if (pcap_findalldevs(alldevs.get(), errbuf) == -1) + if (pcap_findalldevs(alldevs.get(), errbuf.data()) == -1) { - human_readible_error_ = "Error in pcap_findalldevs: " + std::string(errbuf); - fprintf(stderr, "Error in pcap_findalldevs: %s\n", errbuf); + human_readible_error_ = "Error in pcap_findalldevs: " + std::string(errbuf.data()); + fprintf(stderr, "Error in pcap_findalldevs: %s\n", errbuf.data()); return false; } // Check if the loopback device is accessible - for (pcap_if_t* pcap_dev = *alldevs.get(); pcap_dev; pcap_dev = pcap_dev->next) + for (pcap_if_t* pcap_dev = *alldevs.get(); pcap_dev != nullptr; pcap_dev = pcap_dev->next) { if (IsLoopbackDevice_NoLock(pcap_dev->name)) { @@ -256,7 +257,7 @@ namespace Udpcap bool Initialize() { - std::lock_guard npcap_lock(npcap_mutex); + const std::lock_guard npcap_lock(npcap_mutex); if (is_initialized) return true; @@ -296,13 +297,13 @@ namespace Udpcap bool IsInitialized() { - std::lock_guard npcap_lock(npcap_mutex); + const std::lock_guard npcap_lock(npcap_mutex); return is_initialized; } std::string GetLoopbackDeviceUuidString() { - std::lock_guard npcap_lock(npcap_mutex); + const std::lock_guard npcap_lock(npcap_mutex); if (!loopback_device_name_initialized) { @@ -314,7 +315,7 @@ namespace Udpcap std::string GetLoopbackDeviceName() { - std::lock_guard npcap_lock(npcap_mutex); + const std::lock_guard npcap_lock(npcap_mutex); if (!loopback_device_name_initialized) { @@ -330,13 +331,13 @@ namespace Udpcap bool IsLoopbackDevice(const std::string& device_name) { - std::lock_guard npcap_lock(npcap_mutex); + const std::lock_guard npcap_lock(npcap_mutex); return IsLoopbackDevice_NoLock(device_name); } std::string GetHumanReadibleErrorText() { - std::lock_guard npcap_lock(npcap_mutex); + const std::lock_guard npcap_lock(npcap_mutex); return human_readible_error_; } } \ No newline at end of file diff --git a/udpcap/src/udpcap_socket.cpp b/udpcap/src/udpcap_socket.cpp index 1bb767b..066c5a6 100644 --- a/udpcap/src/udpcap_socket.cpp +++ b/udpcap/src/udpcap_socket.cpp @@ -27,8 +27,11 @@ namespace Udpcap : udpcap_socket_private_(std::make_unique()) {} - UdpcapSocket::~UdpcapSocket() - {} + UdpcapSocket::~UdpcapSocket() = default; + + // Move + UdpcapSocket& UdpcapSocket::operator=(UdpcapSocket&&) noexcept = default; + UdpcapSocket::UdpcapSocket(UdpcapSocket&&) noexcept = default; bool UdpcapSocket::isValid () const { return udpcap_socket_private_->isValid(); } diff --git a/udpcap/src/udpcap_socket_private.cpp b/udpcap/src/udpcap_socket_private.cpp index 7293b06..18ce58f 100644 --- a/udpcap/src/udpcap_socket_private.cpp +++ b/udpcap/src/udpcap_socket_private.cpp @@ -40,20 +40,19 @@ namespace Udpcap ////////////////////////////////////////// UdpcapSocketPrivate::UdpcapSocketPrivate() - : is_valid_(false) - , bound_state_(false) - , bound_port_(0) + : is_valid_ (Udpcap::Initialize()) + , bound_state_ (false) + , bound_port_ (0) , multicast_loopback_enabled_(true) - , receive_buffer_size_(-1) + , receive_buffer_size_ (-1) { - is_valid_ = Udpcap::Initialize(); } - UdpcapSocketPrivate::~UdpcapSocketPrivate() - { - // @todo: reinvestigate why it crashes on close. (Maybe check if i have implemented copy / move constructors properly) - //close(); - } + //UdpcapSocketPrivate::~UdpcapSocketPrivate() + //{ + // // @todo: reinvestigate why it crashes on close. (Maybe check if i have implemented copy / move constructors properly) + // //close(); + //} bool UdpcapSocketPrivate::isValid() const { @@ -224,7 +223,7 @@ namespace Udpcap return false; } - if (pcap_win32_handles_.size() < 1) + if (pcap_win32_handles_.empty()) { // No open devices => fail! LOG_DEBUG("Has Pending Datagrams error: No open devices"); @@ -239,7 +238,7 @@ namespace Udpcap num_handles = MAXIMUM_WAIT_OBJECTS; } - DWORD wait_result = WaitForMultipleObjects(num_handles, &pcap_win32_handles_[0], false, 0); + const DWORD wait_result = WaitForMultipleObjects(num_handles, pcap_win32_handles_.data(), static_cast(false), 0); // Check if any HADNLE was in signaled state return((wait_result >= WAIT_OBJECT_0) && wait_result <= (WAIT_OBJECT_0 + num_handles - 1)); @@ -267,7 +266,7 @@ namespace Udpcap return{}; } - if (pcap_win32_handles_.size() < 1) + if (pcap_win32_handles_.empty()) { // No open devices => fail! LOG_DEBUG("Receive error: No open devices"); @@ -281,7 +280,7 @@ namespace Udpcap num_handles = MAXIMUM_WAIT_OBJECTS; } - bool wait_forever = (timeout_ms == INFINITE); + const bool wait_forever = (timeout_ms == INFINITE); auto wait_until = std::chrono::steady_clock::now() + std::chrono::milliseconds(timeout_ms); std::vector datagram; @@ -303,11 +302,11 @@ namespace Udpcap } } - DWORD wait_result = WaitForMultipleObjects(num_handles, &pcap_win32_handles_[0], false, remaining_time_to_wait_ms); + const DWORD wait_result = WaitForMultipleObjects(num_handles, pcap_win32_handles_.data(), static_cast(false), remaining_time_to_wait_ms); if ((wait_result >= WAIT_OBJECT_0) && wait_result <= (WAIT_OBJECT_0 + num_handles - 1)) { - int dev_index = (wait_result - WAIT_OBJECT_0); + const int dev_index = (wait_result - WAIT_OBJECT_0); callback_args.link_type_ = static_cast(pcap_datalink(pcap_devices_[dev_index].pcap_handle_)); callback_args.ip_reassembly_ = ip_reassembly_[dev_index].get(); @@ -358,7 +357,7 @@ namespace Udpcap return{}; } - if (pcap_win32_handles_.size() < 1) + if (pcap_win32_handles_.empty()) { // No open devices => fail! LOG_DEBUG("Receive error: No open devices"); @@ -372,7 +371,7 @@ namespace Udpcap num_handles = MAXIMUM_WAIT_OBJECTS; } - bool wait_forever = (timeout_ms == INFINITE); + const bool wait_forever = (timeout_ms == INFINITE); auto wait_until = std::chrono::steady_clock::now() + std::chrono::milliseconds(timeout_ms); CallbackArgsRawPtr callback_args(data, max_len, source_address, source_port, bound_port_, pcpp::LinkLayerType::LINKTYPE_NULL); @@ -393,11 +392,11 @@ namespace Udpcap } } - DWORD wait_result = WaitForMultipleObjects(num_handles, &pcap_win32_handles_[0], false, remaining_time_to_wait_ms); + const DWORD wait_result = WaitForMultipleObjects(num_handles, pcap_win32_handles_.data(), static_cast(false), remaining_time_to_wait_ms); if ((wait_result >= WAIT_OBJECT_0) && wait_result <= (WAIT_OBJECT_0 + num_handles - 1)) { - int dev_index = (wait_result - WAIT_OBJECT_0); + const int dev_index = (wait_result - WAIT_OBJECT_0); callback_args.link_type_ = static_cast(pcap_datalink(pcap_devices_[dev_index].pcap_handle_)); callback_args.ip_reassembly_ = ip_reassembly_[dev_index].get(); @@ -557,17 +556,17 @@ namespace Udpcap return{}; // Retrieve device list - char errbuf[PCAP_ERRBUF_SIZE]; - pcap_if_t* alldevs_rawptr; - pcap_if_t_uniqueptr alldevs(&alldevs_rawptr, [](pcap_if_t** p) { pcap_freealldevs(*p); }); + std::array errbuf{}; + pcap_if_t* alldevs_rawptr = nullptr; + const pcap_if_t_uniqueptr alldevs(&alldevs_rawptr, [](pcap_if_t** p) { pcap_freealldevs(*p); }); - if (pcap_findalldevs(alldevs.get(), errbuf) == -1) + if (pcap_findalldevs(alldevs.get(), errbuf.data()) == -1) { - fprintf(stderr, "Error in pcap_findalldevs: %s\n", errbuf); + fprintf(stderr, "Error in pcap_findalldevs: %s\n", errbuf.data()); return{}; } - for (pcap_if_t* pcap_dev = *alldevs.get(); pcap_dev; pcap_dev = pcap_dev->next) + for (pcap_if_t* pcap_dev = *alldevs.get(); pcap_dev != nullptr; pcap_dev = pcap_dev->next) { // A user may have done something bad like assigning an IPv4 address to // the loopback adapter. We don't want to open it in that case. In a real- @@ -579,11 +578,11 @@ namespace Udpcap // Iterate through all addresses of the device and check if one of them // matches the one we are looking for. - for (pcap_addr* pcap_dev_addr = pcap_dev->addresses; pcap_dev_addr; pcap_dev_addr = pcap_dev_addr->next) + for (pcap_addr* pcap_dev_addr = pcap_dev->addresses; pcap_dev_addr != nullptr; pcap_dev_addr = pcap_dev_addr->next) { if (pcap_dev_addr->addr->sa_family == AF_INET) { - struct sockaddr_in* device_ipv4_addr = (struct sockaddr_in *)pcap_dev_addr->addr; + struct sockaddr_in* device_ipv4_addr = reinterpret_cast(pcap_dev_addr->addr); if (device_ipv4_addr->sin_addr.s_addr == ip.toInt()) { // The IPv4 address matches! @@ -600,20 +599,20 @@ namespace Udpcap std::vector> UdpcapSocketPrivate::getAllDevices() { // Retrieve device list - char errbuf[PCAP_ERRBUF_SIZE]; - pcap_if_t* alldevs_rawptr; - pcap_if_t_uniqueptr alldevs(&alldevs_rawptr, [](pcap_if_t** p) { pcap_freealldevs(*p); }); + std::array errbuf{}; + pcap_if_t* alldevs_rawptr = nullptr; + const pcap_if_t_uniqueptr alldevs(&alldevs_rawptr, [](pcap_if_t** p) { pcap_freealldevs(*p); }); - if (pcap_findalldevs(alldevs.get(), errbuf) == -1) + if (pcap_findalldevs(alldevs.get(), errbuf.data()) == -1) { - fprintf(stderr, "Error in pcap_findalldevs: %s\n", errbuf); + fprintf(stderr, "Error in pcap_findalldevs: %s\n", errbuf.data()); return{}; } std::vector> alldev_vector; - for (pcap_if_t* pcap_dev = *alldevs.get(); pcap_dev; pcap_dev = pcap_dev->next) + for (pcap_if_t* pcap_dev = *alldevs.get(); pcap_dev != nullptr; pcap_dev = pcap_dev->next) { - alldev_vector.push_back(std::make_pair(std::string(pcap_dev->name), std::string(pcap_dev->description))); + alldev_vector.emplace_back(std::string(pcap_dev->name), std::string(pcap_dev->description)); } return alldev_vector; } @@ -628,7 +627,7 @@ namespace Udpcap std::vector mac(mac_size); // Send OID-Get-Request to the driver - if (pcap_oid_get_request(pcap_handle, OID_802_3_CURRENT_ADDRESS, &mac[0], &mac_size)) + if (pcap_oid_get_request(pcap_handle, OID_802_3_CURRENT_ADDRESS, mac.data(), &mac_size) != 0) { LOG_DEBUG("Error getting MAC address"); return ""; @@ -636,7 +635,7 @@ namespace Udpcap // Convert binary mac into human-readble form (we need it this way for the kernel filter) std::string mac_string(18, ' '); - snprintf(&mac_string[0], mac_string.size(), "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx", mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]); + snprintf(&mac_string[0], mac_string.size(), "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx", mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]); // NOLINT(readability-container-data-pointer) Reason: I need to write to the string, but the data() pointer is const. Since C++11, the operation is safe, as stings are required to be stored in contiguous memory. mac_string.pop_back(); // Remove terminating null char return std::move(mac_string); @@ -650,13 +649,13 @@ namespace Udpcap bool UdpcapSocketPrivate::openPcapDevice(const std::string& device_name) { - char errbuf[PCAP_ERRBUF_SIZE]; + std::array errbuf{}; - pcap_t* pcap_handle = pcap_create(device_name.c_str(), errbuf); + pcap_t* pcap_handle = pcap_create(device_name.c_str(), errbuf.data()); if (pcap_handle == nullptr) { - fprintf(stderr, "\nUnable to open the adapter: %s\n", errbuf); + fprintf(stderr, "\nUnable to open the adapter: %s\n", errbuf.data()); return false; } @@ -669,7 +668,7 @@ namespace Udpcap pcap_set_buffer_size(pcap_handle, receive_buffer_size_); } - int errorcode = pcap_activate(pcap_handle); + const int errorcode = pcap_activate(pcap_handle); switch (errorcode) { case 0: @@ -704,7 +703,7 @@ namespace Udpcap } - PcapDev pcap_dev(pcap_handle, IsLoopbackDevice(device_name), device_name); + const PcapDev pcap_dev(pcap_handle, IsLoopbackDevice(device_name), device_name); pcap_devices_ .push_back(pcap_dev); pcap_win32_handles_.push_back(pcap_getevent(pcap_handle)); @@ -720,7 +719,7 @@ namespace Udpcap // No outgoing packets (determined by MAC, loopback packages don't have an ethernet header) if (!pcap_dev.is_loopback_) { - std::string mac_string = getMac(pcap_dev.pcap_handle_); + const std::string mac_string = getMac(pcap_dev.pcap_handle_); if (!mac_string.empty()) { ss << "not ether src " << mac_string; @@ -744,7 +743,7 @@ namespace Udpcap ss << ")"; // Multicast traffic - if ((multicast_groups_.size() > 0) + if ((!multicast_groups_.empty()) &&(!pcap_dev.is_loopback_ || multicast_loopback_enabled_)) { ss << " or (ip multicast and ("; @@ -765,17 +764,17 @@ namespace Udpcap void UdpcapSocketPrivate::updateCaptureFilter(PcapDev& pcap_dev) { // Create new filter - std::string filter_string = createFilterString(pcap_dev); + const std::string filter_string = createFilterString(pcap_dev); LOG_DEBUG("Setting filter string: " + filter_string); - bpf_program filter_program; + bpf_program filter_program{}; // Compile the filter - int pcap_compile_ret; + int pcap_compile_ret(PCAP_ERROR); { // pcap_compile is not thread safe, so we need a global mutex - std::lock_guard pcap_compile_lock(pcap_compile_mutex); + const std::lock_guard pcap_compile_lock(pcap_compile_mutex); pcap_compile_ret = pcap_compile(pcap_dev.pcap_handle_, &filter_program, filter_string.c_str(), 1, PCAP_NETMASK_UNKNOWN); } @@ -804,13 +803,13 @@ namespace Udpcap void UdpcapSocketPrivate::kickstartLoopbackMulticast() const { - uint16_t kickstart_port = 62000; + const uint16_t kickstart_port = 62000; asio::io_context iocontext; asio::ip::udp::socket kickstart_socket(iocontext); // create socket - asio::ip::udp::endpoint listen_endpoint(asio::ip::make_address("0.0.0.0"), kickstart_port); + const asio::ip::udp::endpoint listen_endpoint(asio::ip::make_address("0.0.0.0"), kickstart_port); kickstart_socket.open(listen_endpoint.protocol()); // set socket reuse @@ -828,7 +827,7 @@ namespace Udpcap // Join all multicast groups for (const auto& multicast_group : multicast_groups_) { - asio::ip::address asio_mc_group = asio::ip::make_address(multicast_group.toString()); + const asio::ip::address asio_mc_group = asio::ip::make_address(multicast_group.toString()); kickstart_socket.set_option(asio::ip::multicast::join_group(asio_mc_group)); } @@ -836,8 +835,8 @@ namespace Udpcap for (const auto& multicast_group : multicast_groups_) { LOG_DEBUG(std::string("Sending loopback kickstart packet to ") + multicast_group.toString() + ":" + std::to_string(kickstart_port)); - asio::ip::address asio_mc_group = asio::ip::make_address(multicast_group.toString()); - asio::ip::udp::endpoint send_endpoint(asio_mc_group, kickstart_port); + const asio::ip::address asio_mc_group = asio::ip::make_address(multicast_group.toString()); + const asio::ip::udp::endpoint send_endpoint(asio_mc_group, kickstart_port); kickstart_socket.send_to(asio::buffer(static_cast(nullptr), 0), send_endpoint, 0); } @@ -849,37 +848,37 @@ namespace Udpcap { CallbackArgsVector* callback_args = reinterpret_cast(param); - pcpp::RawPacket rawPacket(pkt_data, header->caplen, header->ts, false, callback_args->link_type_); - pcpp::Packet packet(&rawPacket, pcpp::UDP); + pcpp::RawPacket rawPacket(pkt_data, header->caplen, header->ts, false, callback_args->link_type_); + const pcpp::Packet packet(&rawPacket, pcpp::UDP); - pcpp::IPv4Layer* ip_layer = packet.getLayerOfType(); - pcpp::UdpLayer* udp_layer = packet.getLayerOfType(); + const pcpp::IPv4Layer* ip_layer = packet.getLayerOfType(); + const pcpp::UdpLayer* udp_layer = packet.getLayerOfType(); - if (ip_layer) + if (ip_layer != nullptr) { if (ip_layer->isFragment()) { // Handle fragmented IP traffic - pcpp::IPReassembly::ReassemblyStatus status; + pcpp::IPReassembly::ReassemblyStatus status(pcpp::IPReassembly::ReassemblyStatus::NON_IP_PACKET); // Try to reassemble packet - pcpp::Packet* reassembled_packet = callback_args->ip_reassembly_->processPacket(&rawPacket, status); + const pcpp::Packet* reassembled_packet = callback_args->ip_reassembly_->processPacket(&rawPacket, status); // If we are done reassembling the packet, we return it to the user - if (reassembled_packet) + if (reassembled_packet != nullptr) { - pcpp::Packet re_parsed_packet(reassembled_packet->getRawPacket(), pcpp::UDP); + const pcpp::Packet re_parsed_packet(reassembled_packet->getRawPacket(), pcpp::UDP); - pcpp::IPv4Layer* reassembled_ip_layer = re_parsed_packet.getLayerOfType(); - pcpp::UdpLayer* reassembled_udp_layer = re_parsed_packet.getLayerOfType(); + const pcpp::IPv4Layer* reassembled_ip_layer = re_parsed_packet.getLayerOfType(); + const pcpp::UdpLayer* reassembled_udp_layer = re_parsed_packet.getLayerOfType(); - if (reassembled_ip_layer && reassembled_udp_layer) + if ((reassembled_ip_layer != nullptr) && (reassembled_udp_layer != nullptr)) FillCallbackArgsVector(callback_args, reassembled_ip_layer, reassembled_udp_layer); delete reassembled_packet; // We need to manually delete the packet pointer } } - else if (udp_layer) + else if (udp_layer != nullptr) { // Handle normal IP traffic (un-fragmented) FillCallbackArgsVector(callback_args, ip_layer, udp_layer); @@ -887,16 +886,16 @@ namespace Udpcap } } - void UdpcapSocketPrivate::FillCallbackArgsVector(CallbackArgsVector* callback_args, pcpp::IPv4Layer* ip_layer, pcpp::UdpLayer* udp_layer) + void UdpcapSocketPrivate::FillCallbackArgsVector(CallbackArgsVector* callback_args, const pcpp::IPv4Layer* ip_layer, const pcpp::UdpLayer* udp_layer) { auto dst_port = ntohs(udp_layer->getUdpHeader()->portDst); if (dst_port == callback_args->bound_port_) { - if (callback_args->source_address_) + if (callback_args->source_address_ != nullptr) *callback_args->source_address_ = HostAddress(ip_layer->getSrcIPv4Address().toInt()); - if (callback_args->source_port_) + if (callback_args->source_port_ != nullptr) *callback_args->source_port_ = ntohs(udp_layer->getUdpHeader()->portSrc); callback_args->destination_vector_->reserve(udp_layer->getLayerPayloadSize()); @@ -909,37 +908,37 @@ namespace Udpcap { CallbackArgsRawPtr* callback_args = reinterpret_cast(param); - pcpp::RawPacket rawPacket(pkt_data, header->caplen, header->ts, false, callback_args->link_type_); - pcpp::Packet packet(&rawPacket, pcpp::UDP); + pcpp::RawPacket rawPacket(pkt_data, header->caplen, header->ts, false, callback_args->link_type_); + const pcpp::Packet packet(&rawPacket, pcpp::UDP); - pcpp::IPv4Layer* ip_layer = packet.getLayerOfType(); - pcpp::UdpLayer* udp_layer = packet.getLayerOfType(); + const pcpp::IPv4Layer* ip_layer = packet.getLayerOfType(); + const pcpp::UdpLayer* udp_layer = packet.getLayerOfType(); - if (ip_layer) + if (ip_layer != nullptr) { if (ip_layer->isFragment()) { // Handle fragmented IP traffic - pcpp::IPReassembly::ReassemblyStatus status; + pcpp::IPReassembly::ReassemblyStatus status(pcpp::IPReassembly::ReassemblyStatus::NON_IP_PACKET); // Try to reasseble packet pcpp::Packet* reassembled_packet = callback_args->ip_reassembly_->processPacket(&rawPacket, status); // If we are done reassembling the packet, we return it to the user - if (reassembled_packet) + if (reassembled_packet != nullptr) { - pcpp::Packet re_parsed_packet(reassembled_packet->getRawPacket(), pcpp::UDP); + const pcpp::Packet re_parsed_packet(reassembled_packet->getRawPacket(), pcpp::UDP); - pcpp::IPv4Layer* reassembled_ip_layer = re_parsed_packet.getLayerOfType(); - pcpp::UdpLayer* reassembled_udp_layer = re_parsed_packet.getLayerOfType(); + const pcpp::IPv4Layer* reassembled_ip_layer = re_parsed_packet.getLayerOfType(); + const pcpp::UdpLayer* reassembled_udp_layer = re_parsed_packet.getLayerOfType(); - if (reassembled_ip_layer && reassembled_udp_layer) + if ((reassembled_ip_layer != nullptr) && (reassembled_udp_layer != nullptr)) FillCallbackArgsRawPtr(callback_args, reassembled_ip_layer, reassembled_udp_layer); delete reassembled_packet; // We need to manually delete the packet pointer } } - else if (udp_layer) + else if (udp_layer != nullptr) { // Handle normal IP traffic (un-fragmented) FillCallbackArgsRawPtr(callback_args, ip_layer, udp_layer); @@ -948,20 +947,20 @@ namespace Udpcap } - void UdpcapSocketPrivate::FillCallbackArgsRawPtr(CallbackArgsRawPtr* callback_args, pcpp::IPv4Layer* ip_layer, pcpp::UdpLayer* udp_layer) + void UdpcapSocketPrivate::FillCallbackArgsRawPtr(CallbackArgsRawPtr* callback_args, const pcpp::IPv4Layer* ip_layer, const pcpp::UdpLayer* udp_layer) { auto dst_port = ntohs(udp_layer->getUdpHeader()->portDst); if (dst_port == callback_args->bound_port_) { - if (callback_args->source_address_) + if (callback_args->source_address_ != nullptr) *callback_args->source_address_ = HostAddress(ip_layer->getSrcIPv4Address().toInt()); - if (callback_args->source_port_) + if (callback_args->source_port_ != nullptr) *callback_args->source_port_ = ntohs(udp_layer->getUdpHeader()->portSrc); - size_t bytes_to_copy = std::min(callback_args->destination_buffer_size_, udp_layer->getLayerPayloadSize()); + const size_t bytes_to_copy = std::min(callback_args->destination_buffer_size_, udp_layer->getLayerPayloadSize()); memcpy_s(callback_args->destination_buffer_, callback_args->destination_buffer_size_, udp_layer->getLayerPayload(), bytes_to_copy); callback_args->bytes_copied_ = bytes_to_copy; diff --git a/udpcap/src/udpcap_socket_private.h b/udpcap/src/udpcap_socket_private.h index 64d046e..f92cfdc 100644 --- a/udpcap/src/udpcap_socket_private.h +++ b/udpcap/src/udpcap_socket_private.h @@ -116,11 +116,16 @@ namespace Udpcap static const int MAX_PACKET_SIZE = 65536; // Npcap Doc: A snapshot length of 65535 should be sufficient, on most if not all networks, to capture all the data available from the packet. UdpcapSocketPrivate(); - ~UdpcapSocketPrivate(); + ~UdpcapSocketPrivate() = default; - UdpcapSocketPrivate(UdpcapSocketPrivate const&) = delete; + // Copy + UdpcapSocketPrivate(UdpcapSocketPrivate const&) = delete; UdpcapSocketPrivate& operator= (UdpcapSocketPrivate const&) = delete; + // Move + UdpcapSocketPrivate& operator=(UdpcapSocketPrivate&&) = default; + UdpcapSocketPrivate(UdpcapSocketPrivate&&) = default; + bool isValid() const; bool bind(const HostAddress& local_address, uint16_t local_port); @@ -169,10 +174,10 @@ namespace Udpcap // Callbacks static void PacketHandlerVector(unsigned char* param, const struct pcap_pkthdr* header, const unsigned char* pkt_data); - static void FillCallbackArgsVector(CallbackArgsVector* callback_args, pcpp::IPv4Layer* ip_layer, pcpp::UdpLayer* udp_layer); + static void FillCallbackArgsVector(CallbackArgsVector* callback_args, const pcpp::IPv4Layer* ip_layer, const pcpp::UdpLayer* udp_layer); static void PacketHandlerRawPtr(unsigned char* param, const struct pcap_pkthdr* header, const unsigned char* pkt_data); - static void FillCallbackArgsRawPtr(CallbackArgsRawPtr* callback_args, pcpp::IPv4Layer* ip_layer, pcpp::UdpLayer* udp_layer); + static void FillCallbackArgsRawPtr(CallbackArgsRawPtr* callback_args, const pcpp::IPv4Layer* ip_layer, const pcpp::UdpLayer* udp_layer); private: bool is_valid_; /**< If the socket is valid and ready to use (e.g. npcap was initialized successfully) */