From 0a8c11be9028969e223bfdd47d0aaccf995b041c Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sun, 25 Aug 2024 09:35:19 +0300 Subject: [PATCH 1/2] Fixes various MSVC compiler warnings. (#1547) * Fixed MSVC C4129 * Fixed MSVC C6262. * Attempt to suppress MSVC C26444 in PTF_ASSERT_RAISES. * Fixed MSVC C26817 * Fixed MSVC C6387 * Fixed MSVC C26478 * Fixed C28182 * Fixed C6297 * Fixes C6011 * Fixed C26439. --- Examples/DnsSpoofing/main.cpp | 7 ++++++- .../IcmpFileTransfer/IcmpFileTransfer-pitcher.cpp | 5 +++++ Packet++/header/HttpLayer.h | 8 ++++---- Packet++/header/SipLayer.h | 8 ++++---- Packet++/src/FtpLayer.cpp | 4 +++- Packet++/src/IPv6Layer.cpp | 5 +++++ Packet++/src/SipLayer.cpp | 4 ++-- Packet++/src/SmtpLayer.cpp | 4 +++- Packet++/src/TextBasedProtocol.cpp | 2 +- Packet++/src/VrrpLayer.cpp | 2 +- Tests/Pcap++Test/Tests/IpMacTests.cpp | 2 +- Tests/Pcap++Test/Tests/LiveDeviceTests.cpp | 13 ++++++++----- Tests/PcppTestFramework/PcppTestFramework.h | 7 +++++++ 13 files changed, 50 insertions(+), 21 deletions(-) diff --git a/Examples/DnsSpoofing/main.cpp b/Examples/DnsSpoofing/main.cpp index eda9912819..3e03c6833a 100644 --- a/Examples/DnsSpoofing/main.cpp +++ b/Examples/DnsSpoofing/main.cpp @@ -6,6 +6,7 @@ * address as the resolved IP. Then it's sent back on the network on the same interface */ +#include #include #include #include @@ -205,11 +206,15 @@ void handleDnsRequest(pcpp::RawPacket* packet, pcpp::PcapLiveDevice* dev, void* ip4Layer->setDstIPv4Address(srcIP.getIPv4()); ip4Layer->getIPv4Header()->ipId = 0; } - else + else if (ip6Layer != nullptr) { ip6Layer->setSrcIPv6Address(ip6Layer->getDstIPv6Address()); ip6Layer->setDstIPv6Address(srcIP.getIPv6()); } + else + { + throw std::logic_error("IPLayer should be either IPv4Layer or IPv6Layer"); + } // reverse src and dst UDP ports uint16_t srcPort = udpLayer->getUdpHeader()->portSrc; diff --git a/Examples/IcmpFileTransfer/IcmpFileTransfer-pitcher.cpp b/Examples/IcmpFileTransfer/IcmpFileTransfer-pitcher.cpp index 2702f3b0c0..67848321fa 100644 --- a/Examples/IcmpFileTransfer/IcmpFileTransfer-pitcher.cpp +++ b/Examples/IcmpFileTransfer/IcmpFileTransfer-pitcher.cpp @@ -7,6 +7,7 @@ */ #include +#include #include #include #ifndef _MSC_VER @@ -37,6 +38,10 @@ void usleep(__int64 usec) ft.QuadPart = -(10 * usec); // Convert to 100 nanosecond interval, negative value indicates relative time timer = CreateWaitableTimer(NULL, TRUE, NULL); + if (timer == nullptr) + { + throw std::runtime_error("Could not create waitable timer with error: " + std::to_string(GetLastError())); + } SetWaitableTimer(timer, &ft, 0, NULL, NULL, 0); WaitForSingleObject(timer, INFINITE); CloseHandle(timer); diff --git a/Packet++/header/HttpLayer.h b/Packet++/header/HttpLayer.h index a63e17dab0..44696be236 100644 --- a/Packet++/header/HttpLayer.h +++ b/Packet++/header/HttpLayer.h @@ -710,13 +710,13 @@ namespace pcpp class HttpRequestFirstLineException : public std::exception { public: - ~HttpRequestFirstLineException() throw() + ~HttpRequestFirstLineException() noexcept {} void setMessage(const std::string& message) { m_Message = message; } - virtual const char* what() const throw() + virtual const char* what() const noexcept { return m_Message.c_str(); } @@ -857,13 +857,13 @@ namespace pcpp class HttpResponseFirstLineException : public std::exception { public: - ~HttpResponseFirstLineException() throw() + ~HttpResponseFirstLineException() noexcept {} void setMessage(const std::string& message) { m_Message = message; } - virtual const char* what() const throw() + virtual const char* what() const noexcept { return m_Message.c_str(); } diff --git a/Packet++/header/SipLayer.h b/Packet++/header/SipLayer.h index 31e1f53d37..90e157d2c7 100644 --- a/Packet++/header/SipLayer.h +++ b/Packet++/header/SipLayer.h @@ -614,13 +614,13 @@ namespace pcpp class SipRequestFirstLineException : public std::exception { public: - ~SipRequestFirstLineException() throw() + ~SipRequestFirstLineException() noexcept {} void setMessage(const std::string& message) { m_Message = message; } - virtual const char* what() const throw() + virtual const char* what() const noexcept { return m_Message.c_str(); } @@ -748,13 +748,13 @@ namespace pcpp class SipResponseFirstLineException : public std::exception { public: - ~SipResponseFirstLineException() throw() + ~SipResponseFirstLineException() noexcept {} void setMessage(const std::string& message) { m_Message = message; } - virtual const char* what() const throw() + virtual const char* what() const noexcept { return m_Message.c_str(); } diff --git a/Packet++/src/FtpLayer.cpp b/Packet++/src/FtpLayer.cpp index f4e500e2ac..dba5828da4 100644 --- a/Packet++/src/FtpLayer.cpp +++ b/Packet++/src/FtpLayer.cpp @@ -17,7 +17,9 @@ namespace pcpp std::string field = getCommandString(); for (size_t idx = 0; idx < field.size(); ++idx) - val |= (field.c_str()[idx] << (idx * 8)); + { + val |= static_cast(field.c_str()[idx]) << (idx * 8); + } return static_cast(val); } diff --git a/Packet++/src/IPv6Layer.cpp b/Packet++/src/IPv6Layer.cpp index 95e56879e3..3819a97b6c 100644 --- a/Packet++/src/IPv6Layer.cpp +++ b/Packet++/src/IPv6Layer.cpp @@ -1,5 +1,6 @@ #define LOG_MODULE PacketLogModuleIPv6Layer +#include #include "IPv6Layer.h" #include "IPv4Layer.h" #include "PayloadLayer.h" @@ -133,6 +134,10 @@ namespace pcpp } else { + if (curExt == nullptr) + { + throw std::logic_error("curExt is nullptr"); + } curExt->setNextHeader(newExt); curExt = curExt->getNextHeader(); } diff --git a/Packet++/src/SipLayer.cpp b/Packet++/src/SipLayer.cpp index d560323975..4e94801e51 100644 --- a/Packet++/src/SipLayer.cpp +++ b/Packet++/src/SipLayer.cpp @@ -360,7 +360,7 @@ namespace pcpp SipRequestLayer::SipRequestLayer(SipMethod method, const std::string& requestUri, const std::string& version) { m_Protocol = SIPRequest; - m_FirstLine = new SipRequestFirstLine(this, method, std::move(version), std::move(requestUri)); + m_FirstLine = new SipRequestFirstLine(this, method, version, requestUri); m_FieldsOffset = m_FirstLine->getSize(); } @@ -598,7 +598,7 @@ namespace pcpp const std::string& sipVersion) { m_Protocol = SIPResponse; - m_FirstLine = new SipResponseFirstLine(this, std::move(sipVersion), statusCode, std::move(statusCodeString)); + m_FirstLine = new SipResponseFirstLine(this, sipVersion, statusCode, std::move(statusCodeString)); m_FieldsOffset = m_FirstLine->getSize(); } diff --git a/Packet++/src/SmtpLayer.cpp b/Packet++/src/SmtpLayer.cpp index dc633c9a2f..4e1c2620cb 100644 --- a/Packet++/src/SmtpLayer.cpp +++ b/Packet++/src/SmtpLayer.cpp @@ -18,7 +18,9 @@ namespace pcpp std::string field = getCommandString(); for (size_t idx = 0; idx < field.size(); ++idx) - val |= (field.c_str()[idx] << (idx * 8)); + { + val |= static_cast(field.c_str()[idx]) << (idx * 8); + } return static_cast(val); } diff --git a/Packet++/src/TextBasedProtocol.cpp b/Packet++/src/TextBasedProtocol.cpp index 7f0f2af246..80f181c47a 100644 --- a/Packet++/src/TextBasedProtocol.cpp +++ b/Packet++/src/TextBasedProtocol.cpp @@ -529,7 +529,7 @@ namespace pcpp { m_NameValueSeparator = nameValueSeparator; m_SpacesAllowedBetweenNameAndValue = spacesAllowedBetweenNameAndValue; - initNewField(std::move(name), std::move(value)); + initNewField(name, value); } void HeaderField::initNewField(const std::string& name, const std::string& value) diff --git a/Packet++/src/VrrpLayer.cpp b/Packet++/src/VrrpLayer.cpp index ac3791ee2a..c56cd7f058 100644 --- a/Packet++/src/VrrpLayer.cpp +++ b/Packet++/src/VrrpLayer.cpp @@ -251,7 +251,7 @@ namespace pcpp size_t ipAddrOffset = 0; uint8_t* newIpAddresses = getData() + offset; - for (auto ipAddress : ipAddresses) + for (auto const& ipAddress : ipAddresses) { copyIPAddressToData(newIpAddresses + ipAddrOffset, ipAddress); ipAddrOffset += ipAddrLen; diff --git a/Tests/Pcap++Test/Tests/IpMacTests.cpp b/Tests/Pcap++Test/Tests/IpMacTests.cpp index 7fa15a470f..84a8c1475f 100644 --- a/Tests/Pcap++Test/Tests/IpMacTests.cpp +++ b/Tests/Pcap++Test/Tests/IpMacTests.cpp @@ -364,7 +364,7 @@ PTF_TEST_CASE(TestGetMacAddress) std::string ipsInArpTableAsString; #ifdef _WIN32 ipsInArpTableAsString = - pcpp::executeShellCommand("arp -a | for /f \"tokens=1\" \%i in ('findstr dynamic') do @echo \%i"); + pcpp::executeShellCommand(R"(arp -a | for /f "tokens=1" %i in ('findstr dynamic') do @echo %i)"); ipsInArpTableAsString.erase(std::remove(ipsInArpTableAsString.begin(), ipsInArpTableAsString.end(), ' '), ipsInArpTableAsString.end()); #else diff --git a/Tests/Pcap++Test/Tests/LiveDeviceTests.cpp b/Tests/Pcap++Test/Tests/LiveDeviceTests.cpp index 1536131312..8f39ab9576 100644 --- a/Tests/Pcap++Test/Tests/LiveDeviceTests.cpp +++ b/Tests/Pcap++Test/Tests/LiveDeviceTests.cpp @@ -11,6 +11,8 @@ #include "../Common/TestUtils.h" #include "../Common/PcapFileNamesDef.h" #include +#include +#include #include #include #if defined(_WIN32) @@ -821,9 +823,8 @@ PTF_TEST_CASE(TestSendPackets) pcpp::PcapFileReaderDevice fileReaderDev(EXAMPLE_PCAP_PATH); PTF_ASSERT_TRUE(fileReaderDev.open()); - pcpp::RawPacket rawPacketArr[10000]; + std::vector rawPacketArr(10000); pcpp::PointerVector packetVec; - pcpp::Packet* packetArr[10000]; int packetsRead = 0; while (fileReaderDev.getNextPacket(rawPacketArr[packetsRead])) { @@ -832,11 +833,13 @@ PTF_TEST_CASE(TestSendPackets) } // send packets as RawPacket array - int packetsSentAsRaw = liveDev->sendPackets(rawPacketArr, packetsRead); + int packetsSentAsRaw = liveDev->sendPackets(rawPacketArr.data(), packetsRead); // send packets as parsed EthPacekt array - std::copy(packetVec.begin(), packetVec.end(), packetArr); - int packetsSentAsParsed = liveDev->sendPackets(packetArr, packetsRead); + std::vector packetArr; + packetArr.reserve(10000); + std::copy(packetVec.begin(), packetVec.end(), std::back_inserter(packetArr)); + int packetsSentAsParsed = liveDev->sendPackets(packetArr.data(), packetsRead); PTF_ASSERT_EQUAL(packetsSentAsRaw, packetsRead); PTF_ASSERT_EQUAL(packetsSentAsParsed, packetsRead); diff --git a/Tests/PcppTestFramework/PcppTestFramework.h b/Tests/PcppTestFramework/PcppTestFramework.h index 341f71d855..9a1e60cad3 100644 --- a/Tests/PcppTestFramework/PcppTestFramework.h +++ b/Tests/PcppTestFramework/PcppTestFramework.h @@ -5,6 +5,12 @@ #include "memplumber.h" #include "PcppTestFrameworkCommon.h" +#ifdef _MSC_VER +# define _PTF_MSVC_SUPPRESS_C26444 _Pragma("warning(suppress: 26444)"); +#else +# define _PTF_MSVC_SUPPRESS_C26444 +#endif // _MSC_VER + #define _PTF_PRINT_TYPE_ACTUAL(exp, val) val #define _PTF_PRINT_TYPE_EXPECTED(exp, val) val #define hex_PTF_PRINT_TYPE_ACTUAL(exp, val) "0x" << std::hex << +val << std::dec @@ -217,6 +223,7 @@ std::string messageCaught = ""; \ try \ { \ + _PTF_MSVC_SUPPRESS_C26444 \ expression; \ } \ catch (const exception_type& e) \ From fa22ff24ba15bf24aa6d26dc15ff39bdd98debef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ege=20=C3=87etin?= <64282645+egecetin@users.noreply.github.com> Date: Sun, 25 Aug 2024 09:47:33 +0300 Subject: [PATCH 2/2] Add new pre-commit hooks (#1543) * add new hooks with fixes * use clang-format inplace flag * mess with formatting * Revert "mess with formatting" This reverts commit fda7c213b105fc69938d3f170f65cf2681e5c06b. * Remove unnecessary hooks --- .pre-commit-config.yaml | 14 ++++++++++++-- Tests/Fuzzers/RegressionTests/run_tests.sh | 10 +++++----- Tests/Fuzzers/ossfuzz.sh | 3 +-- cmake/setup_dpdk.py | 0 4 files changed, 18 insertions(+), 9 deletions(-) mode change 100644 => 100755 cmake/setup_dpdk.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d25b97d4c2..576459527b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -10,11 +10,17 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.6.0 hooks: - - id: trailing-whitespace - id: check-case-conflict + - id: check-executables-have-shebangs + - id: check-json + - id: check-shebang-scripts-are-executable + - id: check-merge-conflict + - id: check-symlinks - id: end-of-file-fixer + - id: forbid-submodules - id: mixed-line-ending args: ['--fix=lf'] + - id: trailing-whitespace - repo: https://github.com/psf/black rev: 24.4.2 hooks: @@ -23,7 +29,7 @@ repos: rev: v1.3.5 hooks: - id: clang-format - args: ["--style=file"] # Use the .clang-format file for configuration + args: ["--style=file", "-i"] # Use the .clang-format file for configuration and apply all fixes files: ^(Common\+\+|Packet\+\+|Pcap\+\+|Tests|Examples)/.*\.(cpp|h)$ - id: cppcheck args: ["--std=c++11", "--language=c++", "--suppressions-list=cppcheckSuppressions.txt", "--inline-suppr", "--force"] @@ -38,3 +44,7 @@ repos: - id: typos args: ['--config=typos-config.toml'] pass_filenames: false + - repo: https://github.com/lovesegfault/beautysh + rev: v6.2.1 + hooks: + - id: beautysh diff --git a/Tests/Fuzzers/RegressionTests/run_tests.sh b/Tests/Fuzzers/RegressionTests/run_tests.sh index 9867e03477..9cf8dbdd11 100755 --- a/Tests/Fuzzers/RegressionTests/run_tests.sh +++ b/Tests/Fuzzers/RegressionTests/run_tests.sh @@ -1,10 +1,10 @@ #!/bin/bash if [ -z "${SAMPLES}" ]; then -SAMPLES=regression_samples + SAMPLES=regression_samples fi if [ -z "${BINARY}" ]; then -BINARY=../Bin/FuzzTarget + BINARY=../Bin/FuzzTarget fi ERR_CODE=0 @@ -14,12 +14,12 @@ GREEN='\033[0;32m' NC='\033[0m' for sample in $(ls ${SAMPLES}); do - echo -n "Running sample $sample..." - "${BINARY}" "$SAMPLES/$sample" &> /dev/null && echo -e "${GREEN}[OK]${NC}" || { FAILED=True && echo -e "${RED}[FAIL]${NC}"; } + echo -n "Running sample $sample..." + "${BINARY}" "$SAMPLES/$sample" &> /dev/null && echo -e "${GREEN}[OK]${NC}" || { FAILED=True && echo -e "${RED}[FAIL]${NC}"; } done if [[ ! -z $FAILED ]]; then - ERR_CODE=1 + ERR_CODE=1 fi exit $ERR_CODE diff --git a/Tests/Fuzzers/ossfuzz.sh b/Tests/Fuzzers/ossfuzz.sh index 0f71ff6240..6d461ff649 100755 --- a/Tests/Fuzzers/ossfuzz.sh +++ b/Tests/Fuzzers/ossfuzz.sh @@ -16,8 +16,7 @@ cmake -DPCAPPP_BUILD_FUZZERS=ON -DPCAPPP_BUILD_TESTS=OFF -DPCAPPP_BUILD_EXAMPLES cmake --build $TARGETS_DIR -j # Copy target and options -FUZZERS=" - FuzzTarget \ +FUZZERS="FuzzTarget \ FuzzTargetNg \ FuzzTargetSnoop \ FuzzWriter \ diff --git a/cmake/setup_dpdk.py b/cmake/setup_dpdk.py old mode 100644 new mode 100755