dump: code cleanup

Change-Id: Ib0978d590dd033ee3e747f5987aa3f63af3d0faf
diff --git a/tools/dump/main.cpp b/tools/dump/main.cpp
index 351d76f..06e9a0e 100644
--- a/tools/dump/main.cpp
+++ b/tools/dump/main.cpp
@@ -35,12 +35,9 @@
  **/
 
 #include "ndndump.hpp"
+#include "core/common.hpp"
 #include "core/version.hpp"
 
-#include <boost/program_options/options_description.hpp>
-#include <boost/program_options/variables_map.hpp>
-#include <boost/program_options/parsers.hpp>
-
 #include <sstream>
 
 namespace ndn {
@@ -51,61 +48,62 @@
 static void
 usage(std::ostream& os, const std::string& appName, const po::options_description& options)
 {
-  os << "Usage:\n"
-     << "  " << appName << " [-i interface] [-f name-filter] [tcpdump-expression] \n"
+  os << "Usage: " << appName << " [options] [pcap-filter]\n"
      << "\n"
-     << "Default tcpdump-expression:\n"
-     << "  '(ether proto 0x8624) || (tcp port 6363) || (udp port 6363)'\n"
-     << "\n";
-  os << options;
+     << "Default pcap-filter:\n"
+     << "    '" << NdnDump::getDefaultPcapFilter() << "'\n"
+     << "\n"
+     << options;
 }
 
 static int
 main(int argc, char* argv[])
 {
-  Ndndump instance;
-  std::string filter;
+  NdnDump instance;
+  std::string nameFilter;
+  std::vector<std::string> pcapFilter;
 
-  po::options_description visibleOptions;
+  po::options_description visibleOptions("Options");
   visibleOptions.add_options()
-    ("help,h", "Produce this help message")
-    ("version,V", "Display version and exit")
+    ("help,h",      "print this help message and exit")
     ("interface,i", po::value<std::string>(&instance.interface),
-                    "Interface from which to dump packets")
-    ("read,r", po::value<std::string>(&instance.inputFile),
-               "Read packets from file")
-    ("verbose,v", "When parsing and printing, produce verbose output")
-    // ("write,w", po::value<std::string>(&instance.outputFile),
-    //  "Write the raw packets to file rather than parsing and printing them out")
-    ("filter,f", po::value<std::string>(&filter),
-                 "Regular expression to filter out Interest and Data packets")
+                    "capture packets from the specified interface; if unspecified, the first "
+                    "non-loopback interface will be used; on Linux, the special value \"any\" "
+                    "can be used to capture from all interfaces")
+    ("read,r",      po::value<std::string>(&instance.inputFile),
+                    "read packets from the specified file; use \"-\" to read from standard input")
+    ("filter,f",    po::value<std::string>(&nameFilter),
+                    "print packet only if name matches this regular expression")
+    ("verbose,v",   po::bool_switch(&instance.isVerbose),
+                    "print more detailed information about each packet")
+    ("version,V",   "print program version and exit")
     ;
 
   po::options_description hiddenOptions;
   hiddenOptions.add_options()
-    ("pcap-program", po::value<std::vector<std::string>>());
+    ("pcap-filter", po::value<std::vector<std::string>>(&pcapFilter))
+    ;
 
-  po::positional_options_description positionalArguments;
-  positionalArguments.add("pcap-program", -1);
+  po::positional_options_description posOptions;
+  posOptions.add("pcap-filter", -1);
 
   po::options_description allOptions;
-  allOptions.add(visibleOptions)
-            .add(hiddenOptions);
+  allOptions.add(visibleOptions).add(hiddenOptions);
 
   po::variables_map vm;
-
   try {
-    po::store(po::command_line_parser(argc, argv)
-                .options(allOptions)
-                .positional(positionalArguments)
-                .run(),
-              vm);
+    po::store(po::command_line_parser(argc, argv).options(allOptions).positional(posOptions).run(), vm);
     po::notify(vm);
   }
   catch (const po::error& e) {
     std::cerr << "ERROR: " << e.what() << "\n\n";
     usage(std::cerr, argv[0], visibleOptions);
-    return 1;
+    return 2;
+  }
+  catch (const boost::bad_any_cast& e) {
+    std::cerr << "ERROR: " << e.what() << "\n\n";
+    usage(std::cerr, argv[0], visibleOptions);
+    return 2;
   }
 
   if (vm.count("help") > 0) {
@@ -114,45 +112,39 @@
   }
 
   if (vm.count("version") > 0) {
-    std::cout << "ndndump " << tools::VERSION << '\n';
+    std::cout << "ndndump " << tools::VERSION << std::endl;
     return 0;
   }
 
-  if (vm.count("verbose") > 0) {
-    instance.isVerbose = true;
+  if (vm.count("interface") > 0 && vm.count("read") > 0) {
+    std::cerr << "ERROR: conflicting '--interface' and '--read' options specified\n\n";
+    usage(std::cerr, argv[0], visibleOptions);
+    return 2;
   }
 
   if (vm.count("filter") > 0) {
     try {
-      instance.nameFilter = std::regex(filter);
+      instance.nameFilter = std::regex(nameFilter);
     }
     catch (const std::regex_error& e) {
-      std::cerr << "ERROR: Invalid filter expression: " << e.what() << std::endl;
+      std::cerr << "ERROR: invalid filter regex: " << e.what() << std::endl;
       return 2;
     }
   }
 
-  if (vm.count("pcap-program") > 0) {
-    const auto& items = vm["pcap-program"].as<std::vector<std::string>>();
-
+  if (vm.count("pcap-filter") > 0) {
     std::ostringstream os;
-    std::copy(items.begin(), items.end(), std::ostream_iterator<std::string>(os, " "));
-    instance.pcapProgram = os.str();
-  }
-
-  if (vm.count("read") > 0 && vm.count("interface") > 0) {
-    std::cerr << "ERROR: Conflicting -r and -i options\n\n";
-    usage(std::cerr, argv[0], visibleOptions);
-    return 2;
+    std::copy(pcapFilter.begin(), pcapFilter.end(), make_ostream_joiner(os, " "));
+    instance.pcapFilter = os.str();
   }
 
   try {
     instance.run();
   }
   catch (const std::exception& e) {
-    std::cerr << "ERROR: " << e.what() << "\n";
+    std::cerr << "ERROR: " << e.what() << std::endl;
+    return 1;
   }
-
   return 0;
 }
 
diff --git a/tools/dump/ndndump.cpp b/tools/dump/ndndump.cpp
index e9a6912..fda16f0 100644
--- a/tools/dump/ndndump.cpp
+++ b/tools/dump/ndndump.cpp
@@ -48,102 +48,123 @@
 
 #include <ndn-cxx/lp/nack.hpp>
 #include <ndn-cxx/lp/packet.hpp>
+#include <ndn-cxx/net/ethernet.hpp>
 
 namespace ndn {
 namespace dump {
 
-const size_t MAX_SNAPLEN = 65535;
-
-Ndndump::Ndndump()
-  : isVerbose(false)
-  , pcapProgram("(ether proto 0x8624) || (tcp port 6363) || (udp port 6363)")
-  // , isSuccinct(false)
-  // , isMatchInverted(false)
-  // , shouldPrintStructure(false)
-  // , isTcpOnly(false)
-  // , isUdpOnly(false)
+NdnDump::~NdnDump()
 {
+  if (m_pcap)
+    pcap_close(m_pcap);
+}
+
+static void
+pcapCallback(uint8_t* user, const pcap_pkthdr* pkthdr, const uint8_t* payload)
+{
+  reinterpret_cast<const NdnDump*>(user)->printPacket(pkthdr, payload);
 }
 
 void
-Ndndump::run()
+NdnDump::run()
 {
-  if (inputFile.empty() && interface.empty()) {
-    char errbuf[PCAP_ERRBUF_SIZE];
-    const char* pcapDevice = pcap_lookupdev(errbuf);
+  char errbuf[PCAP_ERRBUF_SIZE];
 
-    if (pcapDevice == nullptr) {
+  if (inputFile.empty() && interface.empty()) {
+    const char* defaultDevice = pcap_lookupdev(errbuf);
+
+    if (defaultDevice == nullptr) {
       BOOST_THROW_EXCEPTION(Error(errbuf));
     }
 
-    interface = pcapDevice;
+    interface = defaultDevice;
   }
 
-  if (isVerbose) {
-    if (!interface.empty()) {
-      std::cerr << "ndndump: listening on " << interface << std::endl;
-    }
-    else {
-      std::cerr << "ndndump: reading from " << inputFile << std::endl;
-    }
-  }
-
+  std::string action;
   if (!interface.empty()) {
-    char errbuf[PCAP_ERRBUF_SIZE];
-    m_pcap = pcap_open_live(interface.data(), MAX_SNAPLEN, 0, 1000, errbuf);
+    m_pcap = pcap_open_live(interface.data(), 65535, 0, 1000, errbuf);
     if (m_pcap == nullptr) {
-      BOOST_THROW_EXCEPTION(Error("Cannot open interface " + interface + " (" + errbuf + ")"));
+      BOOST_THROW_EXCEPTION(Error("Cannot open interface " + interface + ": " + errbuf));
     }
+    action = "listening on " + interface;
   }
   else {
-    char errbuf[PCAP_ERRBUF_SIZE];
     m_pcap = pcap_open_offline(inputFile.data(), errbuf);
     if (m_pcap == nullptr) {
-      BOOST_THROW_EXCEPTION(Error("Cannot open file " + inputFile + " for reading (" + errbuf + ")"));
+      BOOST_THROW_EXCEPTION(Error("Cannot open file '" + inputFile + "' for reading: " + errbuf));
     }
+    action = "reading from file " + inputFile;
   }
 
-  if (!pcapProgram.empty()) {
+  m_dataLinkType = pcap_datalink(m_pcap);
+  const char* dltName = pcap_datalink_val_to_name(m_dataLinkType);
+  const char* dltDesc = pcap_datalink_val_to_description(m_dataLinkType);
+  std::string formattedDlt = dltName ? dltName : to_string(m_dataLinkType);
+  if (dltDesc) {
+    formattedDlt += " ("s + dltDesc + ")";
+  }
+
+  std::cerr << "ndndump: " << action << ", link-type " << formattedDlt << std::endl;
+
+  switch (m_dataLinkType) {
+  case DLT_EN10MB:
+  case DLT_LINUX_SLL:
+  case DLT_PPP:
+    // we know how to handle these
+    break;
+  default:
+    BOOST_THROW_EXCEPTION(Error("Unsupported link-layer header type " + formattedDlt));
+  }
+
+  if (!pcapFilter.empty()) {
     if (isVerbose) {
-      std::cerr << "ndndump: pcap_filter = " << pcapProgram << std::endl;
+      std::cerr << "ndndump: using pcap filter: " << pcapFilter << std::endl;
     }
 
     bpf_program program;
-    int res = pcap_compile(m_pcap, &program, pcapProgram.data(), 0, PCAP_NETMASK_UNKNOWN);
-
+    int res = pcap_compile(m_pcap, &program, pcapFilter.data(), 0, PCAP_NETMASK_UNKNOWN);
     if (res < 0) {
-      BOOST_THROW_EXCEPTION(Error("Cannot parse tcpdump expression '" + pcapProgram +
-                                  "' (" + pcap_geterr(m_pcap) + ")"));
+      BOOST_THROW_EXCEPTION(Error("Cannot parse pcap filter expression '" + pcapFilter + "': " +
+                                  pcap_geterr(m_pcap)));
     }
 
     res = pcap_setfilter(m_pcap, &program);
     pcap_freecode(&program);
-
     if (res < 0) {
-      BOOST_THROW_EXCEPTION(Error(std::string("pcap_setfilter failed (") + pcap_geterr(m_pcap) + ")"));
+      BOOST_THROW_EXCEPTION(Error("Cannot set pcap filter: "s + pcap_geterr(m_pcap)));
     }
   }
 
-  m_dataLinkType = pcap_datalink(m_pcap);
-  if (m_dataLinkType != DLT_EN10MB && m_dataLinkType != DLT_PPP && m_dataLinkType != DLT_LINUX_SLL) {
-    BOOST_THROW_EXCEPTION(Error("Unsupported pcap format (" + to_string(m_dataLinkType) + ")"));
+  if (pcap_loop(m_pcap, -1, &pcapCallback, reinterpret_cast<uint8_t*>(this)) < 0) {
+    BOOST_THROW_EXCEPTION(Error("pcap_loop: "s + pcap_geterr(m_pcap)));
   }
-
-  pcap_loop(m_pcap, -1, &Ndndump::onCapturedPacket, reinterpret_cast<uint8_t*>(this));
 }
 
 void
-Ndndump::onCapturedPacket(const pcap_pkthdr* header, const uint8_t* packet) const
+NdnDump::printPacket(const pcap_pkthdr* pkthdr, const uint8_t* payload) const
 {
-  std::ostringstream os;
-  printInterceptTime(os, header);
+  // sanity checks
+  if (pkthdr->caplen == 0) {
+    std::cout << "[Invalid header: caplen=0]" << std::endl;
+    return;
+  }
+  if (pkthdr->len == 0) {
+    std::cout << "[Invalid header: len=0]" << std::endl;
+    return;
+  }
+  else if (pkthdr->len < pkthdr->caplen) {
+    std::cout << "[Invalid header: len(" << pkthdr->len
+              << ") < caplen(" << pkthdr->caplen << ")]" << std::endl;
+    return;
+  }
 
-  const uint8_t* payload = packet;
-  ssize_t payloadSize = header->len;
+  std::ostringstream os;
+  printTimestamp(os, pkthdr->ts);
+
+  ssize_t payloadSize = pkthdr->len;
 
   int frameType = skipDataLinkHeaderAndGetFrameType(payload, payloadSize);
   if (frameType < 0) {
-    std::cerr << "Unknown frame type" << std::endl;
     return;
   }
 
@@ -170,7 +191,6 @@
     lpPacket = lp::Packet(block);
 
     Buffer::const_iterator begin, end;
-
     if (lpPacket.has<lp::FragmentField>()) {
       std::tie(begin, end) = lpPacket.get<lp::FragmentField>();
     }
@@ -195,11 +215,9 @@
     if (netPacket.type() == tlv::Interest) {
       Interest interest(netPacket);
       if (matchesFilter(interest.getName())) {
-
         if (lpPacket.has<lp::NackField>()) {
           lp::Nack nack(interest);
           nack.setHeader(lpPacket.get<lp::NackField>());
-
           std::cout << os.str() << ", " << "NACK: " << nack.getReason() << ", " << interest << std::endl;
         }
         else {
@@ -223,43 +241,30 @@
 }
 
 void
-Ndndump::printInterceptTime(std::ostream& os, const pcap_pkthdr* header) const
+NdnDump::printTimestamp(std::ostream& os, const timeval& tv) const
 {
-  os << header->ts.tv_sec
+  /// \todo Add more timestamp formats (time since previous packet, time since first packet, ...)
+  os << tv.tv_sec
      << "."
-     << std::setfill('0') << std::setw(6) << header->ts.tv_usec;
-
-  // struct tm* tm;
-  // if (flags.unit_time) {
-  //   os << (int) header->ts.tv_sec
-  //      << "."
-  //      << setfill('0') << setw(6) << (int)header->ts.tv_usec;
-  // }
-  // else {
-  //   tm = localtime(&(header->ts.tv_sec));
-  //   os << (int)tm->tm_hour << ":"
-  //      << setfill('0') << setw(2) << (int)tm->tm_min<< ":"
-  //      << setfill('0') << setw(2) << (int)tm->tm_sec<< "."
-  //      << setfill('0') << setw(6) << (int)header->ts.tv_usec;
-  // }
-  os << " ";
+     << std::setfill('0') << std::setw(6) << tv.tv_usec
+     << " ";
 }
 
 int
-Ndndump::skipDataLinkHeaderAndGetFrameType(const uint8_t*& payload, ssize_t& payloadSize) const
+NdnDump::skipDataLinkHeaderAndGetFrameType(const uint8_t*& payload, ssize_t& payloadSize) const
 {
-  int frameType = 0;
+  int frameType = -1;
 
   switch (m_dataLinkType) {
     case DLT_EN10MB: { // Ethernet frames can have Ethernet or 802.3 encapsulation
-      const ether_header* etherHeader = reinterpret_cast<const ether_header*>(payload);
+      const ether_header* eh = reinterpret_cast<const ether_header*>(payload);
 
-      if (payloadSize < 0) {
-        std::cerr << "Invalid pcap Ethernet frame" << std::endl;
+      if (payloadSize < ETHER_HDR_LEN) {
+        std::cerr << "Invalid Ethernet frame" << std::endl;
         return -1;
       }
 
-      frameType = ntohs(etherHeader->ether_type);
+      frameType = ntohs(eh->ether_type);
       payloadSize -= ETHER_HDR_LEN;
       payload += ETHER_HDR_LEN;
 
@@ -276,7 +281,7 @@
         ++payload;
       }
 
-      if (payloadSize < 0) {
+      if (payloadSize < 4) { // PPP_HDRLEN in linux/ppp_defs.h
         std::cerr << "Invalid PPP frame" << std::endl;
         return -1;
       }
@@ -284,36 +289,38 @@
       break;
     }
     case DLT_LINUX_SLL: {
-      const sll_header* sllHeader = reinterpret_cast<const sll_header*>(payload);
+      const sll_header* sll = reinterpret_cast<const sll_header*>(payload);
 
       if (payloadSize < SLL_HDR_LEN) {
         std::cerr << "Invalid LINUX_SLL frame" << std::endl;
         return -1;
       }
 
-      frameType = ntohs(sllHeader->sll_protocol);
+      frameType = ntohs(sll->sll_protocol);
       payloadSize -= SLL_HDR_LEN;
       payload += SLL_HDR_LEN;
 
       break;
     }
+    default:
+      std::cerr << "Unknown frame type" << std::endl;
+      break;
   }
 
   return frameType;
 }
 
 int
-Ndndump::skipAndProcessFrameHeader(int frameType,
-                                   const uint8_t*& payload, ssize_t& payloadSize,
+NdnDump::skipAndProcessFrameHeader(int frameType, const uint8_t*& payload, ssize_t& payloadSize,
                                    std::ostream& os) const
 {
   switch (frameType) {
-    case 0x0800: // ETHERTYPE_IP
+    case ETHERTYPE_IP:
     case DLT_EN10MB: { // pcap encapsulation
       const ip* ipHeader = reinterpret_cast<const ip*>(payload);
       size_t ipHeaderSize = ipHeader->ip_hl * 4;
       if (ipHeaderSize < 20) {
-        std::cerr << "invalid IP header len " << ipHeaderSize << " bytes" << std::endl;
+        std::cerr << "Invalid IPv4 header len: " << ipHeaderSize << " bytes" << std::endl;
         return -1;
       }
 
@@ -324,20 +331,17 @@
       payload += ipHeaderSize;
 
       if (payloadSize < 0) {
-        std::cerr << "Invalid pcap IP packet" << std::endl;
+        std::cerr << "Invalid IPv4 packet" << std::endl;
         return -1;
       }
 
       switch (ipHeader->ip_p) {
         case IPPROTO_UDP: {
-          // if (!flags.udp)
-          //   return -1;
-
           payloadSize -= sizeof(udphdr);
           payload += sizeof(udphdr);
 
           if (payloadSize < 0) {
-            std::cerr << "Invalid pcap UDP/IP packet" << std::endl;
+            std::cerr << "Invalid UDP/IP packet" << std::endl;
             return -1;
           }
 
@@ -345,14 +349,11 @@
           break;
         }
         case IPPROTO_TCP: {
-          // if (!flags.tcp)
-          //   return -1;
-
           const tcphdr* tcpHeader = reinterpret_cast<const tcphdr*>(payload);
           size_t tcpHeaderSize = tcpHeader->th_off * 4;
 
           if (tcpHeaderSize < 20) {
-            std::cerr << "Invalid TCP Header len: " << tcpHeaderSize <<" bytes" << std::endl;
+            std::cerr << "Invalid TCP header len: " << tcpHeaderSize << " bytes" << std::endl;
             return -1;
           }
 
@@ -360,7 +361,7 @@
           payload += tcpHeaderSize;
 
           if (payloadSize < 0) {
-            std::cerr << "Invalid pcap TCP/IP packet" << std::endl;
+            std::cerr << "Invalid TCP/IP packet" << std::endl;
             return -1;
           }
 
@@ -370,16 +371,13 @@
         default:
           return -1;
       }
-
       break;
     }
-    case /*ETHERTYPE_NDN*/0x7777:
+    case ethernet::ETHERTYPE_NDN:
+    case 0x7777: // NDN ethertype used in ndnSIM
       os << "Tunnel Type: EthernetFrame";
       break;
-    case /*ETHERTYPE_NDNLP*/0x8624:
-      os << "Tunnel Type: EthernetFrame";
-      break;
-    case 0x0077: // pcap
+    case 0x0077: // protocol field in PPP header used in ndnSIM
       os << "Tunnel Type: PPP";
       payloadSize -= 2;
       payload += 2;
@@ -392,7 +390,7 @@
 }
 
 bool
-Ndndump::matchesFilter(const Name& name) const
+NdnDump::matchesFilter(const Name& name) const
 {
   if (!nameFilter)
     return true;
diff --git a/tools/dump/ndndump.hpp b/tools/dump/ndndump.hpp
index bf1ff55..7b5e240 100644
--- a/tools/dump/ndndump.hpp
+++ b/tools/dump/ndndump.hpp
@@ -41,73 +41,59 @@
 
 #include <pcap.h>
 #include <regex>
-#include <ndn-cxx/name.hpp>
 
 namespace ndn {
 namespace dump {
 
-class Ndndump : noncopyable
+class NdnDump : noncopyable
 {
 public:
   class Error : public std::runtime_error
   {
   public:
-    explicit
-    Error(const std::string& what)
-      : std::runtime_error(what)
-    {
-    }
+    using std::runtime_error::runtime_error;
   };
 
-  Ndndump();
+  ~NdnDump();
 
   void
   run();
 
-PUBLIC_WITH_TESTS_ELSE_PRIVATE:
   void
-  onCapturedPacket(const pcap_pkthdr* header, const uint8_t* packet) const;
+  printPacket(const pcap_pkthdr* pkthdr, const uint8_t* payload) const;
 
-private:
-  static void
-  onCapturedPacket(uint8_t* userData, const pcap_pkthdr* header, const uint8_t* packet)
+  static constexpr const char*
+  getDefaultPcapFilter() noexcept
   {
-    reinterpret_cast<const Ndndump*>(userData)->onCapturedPacket(header, packet);
+    return "(ether proto 0x8624) or (tcp port 6363) or (udp port 6363)";
   }
 
+private:
   void
-  printInterceptTime(std::ostream& os, const pcap_pkthdr* header) const;
+  printTimestamp(std::ostream& os, const timeval& tv) const;
 
   int
   skipDataLinkHeaderAndGetFrameType(const uint8_t*& payload, ssize_t& payloadSize) const;
 
   int
-  skipAndProcessFrameHeader(int frameType,
-                            const uint8_t*& payload, ssize_t& payloadSize,
+  skipAndProcessFrameHeader(int frameType, const uint8_t*& payload, ssize_t& payloadSize,
                             std::ostream& os) const;
 
   bool
   matchesFilter(const Name& name) const;
 
-public:
-  bool isVerbose;
-  // bool isSuccinct;
-  // bool isMatchInverted;
-  // bool shouldPrintStructure;
-  // bool isTcpOnly;
-  // bool isUdpOnly;
-
-  std::string pcapProgram;
+public: // options
   std::string interface;
-  optional<std::regex> nameFilter;
   std::string inputFile;
-  // std::string outputFile;
+  std::string pcapFilter = getDefaultPcapFilter();
+  optional<std::regex> nameFilter;
+  bool isVerbose = false;
 
 private:
-  pcap_t* m_pcap;
+  pcap_t* m_pcap = nullptr;
 
 PUBLIC_WITH_TESTS_ELSE_PRIVATE:
-  int m_dataLinkType;
+  int m_dataLinkType = -1;
 };
 
 } // namespace dump