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.
seladb · Jun 13, 2024 · 78ef4e6 · 78ef4e6
1 parent f750de9
commit 78ef4e6
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 27 deletions.
diff --git 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
@@ -4,6 +4,9 @@
 #ifndef _MSC_VER
 #include <unistd.h>
 #endif
+#include <stdexcept>
+#include <memory>
+#include <array>
 #include <iostream>
 #include <mutex>
 #include <signal.h>
@@ -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<SystemCore>& 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<FILE, PcloseDeleter> pipe = std::unique_ptr<FILE, PcloseDeleter>(POPEN(command.c_str(), "r"));
+	if (!pipe)
+	{
+		throw std::runtime_error("Error executing command: " + command);
+	}
+
+	std::array<char, 128> 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
@@ -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
@@ -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 <PcapPlusPlus_Root>/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 <PcapPlusPlus_Root>/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
@@ -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 <PF_RING_LOCATION>/kernel/pf_ring.ko'");
 		return;