Separate IPMI and legacy snoop code to different files
snoopd contains two implementations that have very little in common.
Separate the IPMI code (functions and global objects) into its own file
to reduce #ifdefs and make it easier to understand. Better separate the
cmdline options and help text to only be used in the applicable build
configuration.
Tested: Built with -Dsnoop=enabled and verified only -h option is
accepted. Built without -Dsnoop and verified -d/-r/-b options are
accepted.
Change-Id: I7f14469fafd9f050f6b9f356882ce9e26fb7f206
Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
diff --git a/main.cpp b/main.cpp
index 53a075e..2c9c7c2 100644
--- a/main.cpp
+++ b/main.cpp
@@ -17,7 +17,6 @@
#ifdef ENABLE_IPMI_SNOOP
#include "ipmisnoop/ipmisnoop.hpp"
#endif
-
#include "lpcsnoop/snoop.hpp"
#include <endian.h>
@@ -41,78 +40,26 @@
#include <functional>
#include <iostream>
#include <optional>
-#include <span>
#include <thread>
-#ifdef ENABLE_IPMI_SNOOP
-#include <xyz/openbmc_project/State/Boot/Raw/server.hpp>
-#endif
-
static size_t codeSize = 1; /* Size of each POST code in bytes */
-const char* defaultHostInstances = "0";
static bool verbose = false;
-#ifdef ENABLE_IPMI_SNOOP
-const uint8_t minPositionVal = 0;
-const uint8_t maxPositionVal = 5;
-#endif
-
-#ifdef ENABLE_IPMI_SNOOP
-std::vector<std::unique_ptr<IpmiPostReporter>> reporters;
-#endif
-
-#ifdef ENABLE_IPMI_SNOOP
-void IpmiPostReporter::getSelectorPositionSignal(sdbusplus::bus_t& bus)
-{
- size_t posVal = 0;
-
- matchSignal = std::make_unique<sdbusplus::bus::match_t>(
- bus,
- sdbusplus::bus::match::rules::propertiesChanged(selectorObject,
- selectorIface),
- [&](sdbusplus::message_t& msg) {
- std::string objectName;
- std::map<std::string, Selector::PropertiesVariant> msgData;
- msg.read(objectName, msgData);
-
- auto valPropMap = msgData.find("Position");
- {
- if (valPropMap == msgData.end())
- {
- std::cerr << "Position property not found " << std::endl;
- return;
- }
-
- posVal = std::get<size_t>(valPropMap->second);
-
- if (posVal > minPositionVal && posVal < maxPositionVal)
- {
- std::tuple<uint64_t, secondary_post_code_t> postcodes =
- reporters[posVal - 1]->value();
- uint64_t postcode = std::get<uint64_t>(postcodes);
-
- // write postcode into seven segment display
- if (postCodeDisplay(postcode) < 0)
- {
- fprintf(stderr, "Error in display the postcode\n");
- }
- }
- }
- });
-}
-#endif
static void usage(const char* name)
{
fprintf(stderr,
- "Usage: %s [-d <DEVICE>]\n"
- " -b, --bytes <SIZE> set POST code length to <SIZE> bytes. "
- "Default is %zu\n"
+ "Usage: %s\n"
+#ifdef ENABLE_IPMI_SNOOP
+ " -h, --host <host instances> Default is '0'\n"
+#else
" -d, --device <DEVICE> use <DEVICE> file.\n"
" -r, --rate-limit=<N> Only process N POST codes from the "
"device per second.\n"
- " -h, --host <host instances> . Default is '%s'\n"
+ " -b, --bytes <SIZE> set POST code length to <SIZE> bytes. "
+ "Default is 1\n"
+#endif
" -v, --verbose Prints verbose information while running\n\n",
- name, codeSize, defaultHostInstances);
+ name);
}
/**
@@ -225,66 +172,6 @@
s.get_event().exit(1);
}
-#ifdef ENABLE_IPMI_SNOOP
-// handle muti-host D-bus
-int postCodeIpmiHandler(const std::string& snoopObject,
- const std::string& snoopDbus, sdbusplus::bus_t& bus,
- std::span<std::string> host)
-{
- int ret = 0;
-
- try
- {
- for (size_t iteration = 0; iteration < host.size(); iteration++)
- {
- std::string objPathInst = snoopObject + host[iteration];
-
- sdbusplus::server::manager_t m{bus, objPathInst.c_str()};
-
- /* Create a monitor object and let it do all the rest */
- reporters.emplace_back(
- std::make_unique<IpmiPostReporter>(bus, objPathInst.c_str()));
-
- reporters[iteration]->emit_object_added();
- }
-
- bus.request_name(snoopDbus.c_str());
-
- /* sevenSegmentLedEnabled flag is unset when GPIO pins are not there 7
- seg display for fewer platforms. So, the code for postcode dispay and
- Get Selector position can be skipped in those platforms.
- */
- if (sevenSegmentLedEnabled)
- {
- reporters[0]->getSelectorPositionSignal(bus);
- }
- else
- {
- reporters.clear();
- }
- }
- catch (const std::exception& e)
- {
- fprintf(stderr, "%s\n", e.what());
- }
-
- // Configure seven segment dsiplay connected to GPIOs as output
- ret = configGPIODirOutput();
- if (ret < 0)
- {
- fprintf(stderr, "Failed find the gpio line. Cannot display postcodes "
- "in seven segment display..\n");
- }
-
- while (true)
- {
- bus.process_discard();
- bus.wait();
- }
- exit(EXIT_SUCCESS);
-}
-#endif
-
/*
* TODO(venture): this only listens one of the possible snoop ports, but
* doesn't share the namespace.
@@ -294,47 +181,41 @@
*/
int main(int argc, char* argv[])
{
-#ifndef ENABLE_IPMI_SNOOP
int postFd = -1;
unsigned int rateLimit = 0;
-#endif
int opt;
-#ifdef ENABLE_IPMI_SNOOP
std::vector<std::string> host;
-#endif
- /*
- * These string constants are only used in this method within this object
- * and this object is the only object feeding into the final binary.
- *
- * If however, another object is added to this binary it would be proper
- * to move these declarations to be global and extern to the other object.
- */
// clang-format off
static const struct option long_options[] = {
- #ifdef ENABLE_IPMI_SNOOP
+#ifdef ENABLE_IPMI_SNOOP
{"host", optional_argument, NULL, 'h'},
- #endif
- {"bytes", required_argument, NULL, 'b'},
- #ifndef ENABLE_IPMI_SNOOP
+#else
{"device", optional_argument, NULL, 'd'},
{"rate-limit", optional_argument, NULL, 'r'},
- #endif
+ {"bytes", required_argument, NULL, 'b'},
+#endif
{"verbose", no_argument, NULL, 'v'},
{0, 0, 0, 0}
};
// clang-format on
- while ((opt = getopt_long(argc, argv, "h:b:d:r:v", long_options, NULL)) !=
- -1)
+ constexpr const char* optstring =
+#ifdef ENABLE_IPMI_SNOOP
+ "h:"
+#else
+ "d:r:b:"
+#endif
+ "v";
+
+ while ((opt = getopt_long(argc, argv, optstring, long_options, NULL)) != -1)
{
switch (opt)
{
case 0:
break;
-#ifdef ENABLE_IPMI_SNOOP
case 'h':
{
std::string_view instances = optarg;
@@ -348,7 +229,6 @@
host.emplace_back(instances);
break;
}
-#endif
case 'b':
{
codeSize = atoi(optarg);
@@ -363,7 +243,6 @@
}
break;
}
-#ifndef ENABLE_IPMI_SNOOP
case 'd':
postFd = open(optarg, O_NONBLOCK);
@@ -395,7 +274,6 @@
argVal);
break;
}
-#endif
case 'v':
verbose = true;
break;
@@ -418,8 +296,6 @@
return 0;
#endif
-#ifndef ENABLE_IPMI_SNOOP
-
bool deferSignals = true;
// Add systemd object manager.
@@ -464,5 +340,4 @@
}
return 0;
-#endif
}