From 78ef4e6a4ae283de92a2c86e38380158566c2d05 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Thu, 13 Jun 2024 13:45:26 +0300 Subject: [PATCH] Refactor executeShellCommand to throw exceptions. (#1446) * Refactored executeShellCommand to throw a runtime_error exception if the command fails instead of returning a magic "ERROR" string. * Forwarded command execution to 'executeShellCommand'. * Lint * Fixed log message. * Changed raw array to std::array. * Moved PcloseDeleter to be in a top-level unnamed namespace. Added docstring to PcloseDeleter. --- Common++/header/SystemUtils.h | 3 ++- Common++/src/SystemUtils.cpp | 40 +++++++++++++++++++++++++-------- Examples/KniPong/main.cpp | 11 +++++++-- Pcap++/src/DpdkDeviceList.cpp | 25 +++++++++++---------- Pcap++/src/PfRingDeviceList.cpp | 17 +++++++++++--- 5 files changed, 69 insertions(+), 27 deletions(-) diff --git a/Common++/header/SystemUtils.h b/Common++/header/SystemUtils.h index d63ba20733..ad03d1dec8 100644 --- a/Common++/header/SystemUtils.h +++ b/Common++/header/SystemUtils.h @@ -228,8 +228,9 @@ namespace pcpp * Execute a shell command and return its output * @param[in] command The command to run * @return The output of the command (both stdout and stderr) + * @throws std::runtime_error Error executing the command. */ - std::string executeShellCommand(const std::string &command); + std::string executeShellCommand(const std::string& command); /** * Check if a directory exists diff --git a/Common++/src/SystemUtils.cpp b/Common++/src/SystemUtils.cpp index 0bc9fdf0d0..85dbbc0d37 100644 --- a/Common++/src/SystemUtils.cpp +++ b/Common++/src/SystemUtils.cpp @@ -4,6 +4,9 @@ #ifndef _MSC_VER #include #endif +#include +#include +#include #include #include #include @@ -48,6 +51,22 @@ int gettimeofday(struct timeval * tp, struct timezone * tzp) } #endif +/// @cond PCPP_INTERNAL + +namespace +{ + /** + * @class PcloseDeleter + * A deleter that cleans up a FILE handle using pclose. + */ + struct PcloseDeleter + { + void operator()(FILE* ptr) const { PCLOSE(ptr); } + }; +} // namespace + +/// @endcond + namespace pcpp { @@ -183,18 +202,21 @@ void createCoreVectorFromCoreMask(CoreMask coreMask, std::vector& re } } -std::string executeShellCommand(const std::string &command) +std::string executeShellCommand(const std::string& command) { - FILE* pipe = POPEN(command.c_str(), "r"); - if (!pipe) return "ERROR"; - char buffer[128]; - std::string result = ""; - while(!feof(pipe)) + std::unique_ptr pipe = std::unique_ptr(POPEN(command.c_str(), "r")); + if (!pipe) + { + throw std::runtime_error("Error executing command: " + command); + } + + std::array buffer; + std::string result; + while(!feof(pipe.get())) { - if(fgets(buffer, 128, pipe) != nullptr) - result += buffer; + if(fgets(buffer.data(), buffer.size(), pipe.get()) != nullptr) + result += buffer.data(); // Using the C-string overload of string append. } - PCLOSE(pipe); return result; } diff --git a/Examples/KniPong/main.cpp b/Examples/KniPong/main.cpp index 35d0c05fa1..c03bb187d0 100644 --- a/Examples/KniPong/main.cpp +++ b/Examples/KniPong/main.cpp @@ -241,8 +241,15 @@ inline bool setKniIp(const pcpp::IPv4Address& ip, const std::string& kniName) pcpp::executeShellCommand(command.str()); command.str(""); command << "ip a | grep " << ip; - std::string result = pcpp::executeShellCommand(command.str()); - return result != "" && result != "ERROR"; + try + { + std::string result = pcpp::executeShellCommand(command.str()); + return result != ""; + } + catch (const std::runtime_error&) + { + return false; + } } diff --git a/Pcap++/src/DpdkDeviceList.cpp b/Pcap++/src/DpdkDeviceList.cpp index 2dbe167b95..f2b139129b 100644 --- a/Pcap++/src/DpdkDeviceList.cpp +++ b/Pcap++/src/DpdkDeviceList.cpp @@ -258,24 +258,25 @@ bool DpdkDeviceList::verifyHugePagesAndDpdkDriver() execResult = executeShellCommand("lsmod | grep -s igb_uio"); if (execResult == "") { - execResult = executeShellCommand("modinfo -d uio_pci_generic"); - if (execResult.find("ERROR") != std::string::npos) + try { - execResult = executeShellCommand("modinfo -d vfio-pci"); - if (execResult.find("ERROR") != std::string::npos) + execResult = executeShellCommand("modinfo -d uio_pci_generic"); + PCPP_LOG_DEBUG("uio_pci_generic module is loaded"); + } + catch (const std::runtime_error&) + { + try { - PCPP_LOG_ERROR("None of igb_uio, uio_pci_generic, vfio-pci kernel modules are loaded so DPDK cannot be initialized. Please run /setup_dpdk.sh"); - return false; + execResult = executeShellCommand("modinfo -d vfio-pci"); + PCPP_LOG_DEBUG("vfio-pci module is loaded"); } - else + catch (const std::runtime_error&) { - PCPP_LOG_DEBUG("vfio-pci module is loaded"); + PCPP_LOG_ERROR("None of igb_uio, uio_pci_generic, vfio-pci kernel modules are loaded so DPDK cannot be " + "initialized. Please run /setup_dpdk.sh"); + return false; } } - else - { - PCPP_LOG_DEBUG("uio_pci_generic module is loaded"); - } } else PCPP_LOG_DEBUG("igb_uio driver is loaded"); diff --git a/Pcap++/src/PfRingDeviceList.cpp b/Pcap++/src/PfRingDeviceList.cpp index 8102f901a3..5a26b3f74f 100644 --- a/Pcap++/src/PfRingDeviceList.cpp +++ b/Pcap++/src/PfRingDeviceList.cpp @@ -5,6 +5,7 @@ #define LOG_MODULE PcapLogModulePfRingDevice #include "PfRingDeviceList.h" +#include "SystemUtils.h" #include "Logger.h" #include "pcap.h" #include "pfring.h" @@ -16,9 +17,19 @@ PfRingDeviceList::PfRingDeviceList() { m_PfRingVersion = ""; - FILE *fd = popen("lsmod | grep pf_ring", "r"); - char buf[16]; - if (!fread(buf, 1, sizeof (buf), fd)) // if there is some result the module must be loaded + bool moduleLoaded = false; + try + { + // if there is some result the module must be loaded + moduleLoaded = !(executeShellCommand("lsmod | grep pf_ring").empty()); + } + catch (const std::exception& e) + { + PCPP_LOG_ERROR("PF_RING load error: " << e.what()); + moduleLoaded = false; + } + + if (!moduleLoaded) { PCPP_LOG_ERROR("PF_RING kernel module isn't loaded. Please run: 'sudo insmod /kernel/pf_ring.ko'"); return;