Cleanup buses map container, to really contains the UsbBus object

Issues that existed in 2012 do not exist anymore!

CMakeLists.txt

@@ -8,7 +8,6 @@ set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CMAKE_SOURCE_DIR}/cmake)
 
 # PCAP
 find_package(PCAP REQUIRED)
-find_package(Boost REQUIRED COMPONENTS thread)
 
 include_directories(include)
 

include/usbtop/buses.h

@@ -38,10 +38,7 @@ namespace usbtop {
 class UsbBuses
 {
 	typedef void(*bus_func_t)(UsbBus::id_type bus_id, const char* name, const char* desc);
-	typedef std::map<UsbBus::id_type, UsbBus*> list_buses_t;
-
-public:
-	~UsbBuses();
+	typedef std::map<UsbBus::id_type, UsbBus> list_buses_t;
 
 public:
 	static void list(bus_func_t f, const char* filter);
@@ -53,7 +50,7 @@ class UsbBuses
 		populate();
 		list_buses_t::iterator it;
 		for (it = _buses.begin(); it != _buses.end(); it++) {
-			UsbBus* bus = it->second;
+			UsbBus* bus = &it->second;
 			if (!filter || (filter && bus->name() == filter)) {
 				nfiltered++;
 				f(bus);
@@ -73,15 +70,6 @@ class UsbBuses
 	static bool _populated;
 
 private:
-	// AG: ideally, we shouldn't store a pointer into this std::map,
-	// because "emplace" can be use to create non-copyable object into
-	// a container.
-	// Two issues here: std::map::emplace isn't implemented in libstdc++
-	// (as of August 2012), and, even with std::unordered_map, the only
-	// supported syntax is :
-	// map.emplace(key, UsbBus(...)), which involves a copy of the UsbBus object... (really usefull).
-	// The best would be to do:
-	// map.emplace(key, args_for_usb_bus)
 	static list_buses_t _buses;
 };
 

include/usbtop/usb_stats.h

@@ -35,8 +35,6 @@
 #include <cstddef>
 
 #include <boost/circular_buffer.hpp>
-#include <boost/noncopyable.hpp>
-#include <boost/thread/shared_mutex.hpp>
 
 #define LIVE_SAMPLE_COUNT 128
 
@@ -48,10 +46,10 @@ class Stats: boost::noncopyable
 public:
 	Stats();
 	Stats(Stats&& o):
-		_nbytes(o._nbytes),
-		_tN(o._tN),
-		_nsamples(o._nsamples),
-		_inst_data(std::move(o._inst_data))
+		nbytes_(o.nbytes_),
+		tN_(o.tN_),
+		nsamples_(o.nsamples_),
+		inst_data_(std::move(o.inst_data_))
 	{ }
 
 public:
@@ -64,22 +62,21 @@ class Stats: boost::noncopyable
 
 private:
 	// Global stats
-	size_t _nbytes;
-	double _tN;
-	size_t _nsamples;
+	size_t nbytes_;
+	double tN_;
+	size_t nsamples_;
 
 	// "Instantaneous" stats
-	boost::circular_buffer<sample_t> _inst_data;
-	mutable boost::shared_mutex _access;
+	boost::circular_buffer<sample_t> inst_data_;
 
 	// Timestamp when the application is launched. Used as t0
-	static double _t0;
+	static double t0_;
 
 	// Last "instantaneous" stats
-	double _last_inst_bw;
+	double last_inst_bw_;
 
 	// Time window for statistics in seconds
-	double _stats_window;
+	double stats_window_;
 };
 
 class UsbStats: boost::noncopyable

src/CMakeLists.txt

@@ -14,6 +14,6 @@ usb_stats.cpp
 )
 
 add_executable(usbtop ${SRC_FILES})
-target_link_libraries(usbtop ${PCAP_LIBRARIES} Boost::thread) 
+target_link_libraries(usbtop ${PCAP_LIBRARIES} pthread)
 
 install(TARGETS usbtop DESTINATION sbin)

src/buses.cpp

@@ -40,88 +40,82 @@
 
 #define USB_DEVICE_START "usbmon"
 
-static size_t g_len_usb_dev_start = 5; // strlen(USB_DEVICE_START
+static size_t g_len_usb_dev_start = strlen(USB_DEVICE_START);
 
-std::map<usbtop::UsbBus::id_type, usbtop::UsbBus*> usbtop::UsbBuses::_buses;
+std::map<usbtop::UsbBus::id_type, usbtop::UsbBus> usbtop::UsbBuses::_buses;
 bool usbtop::UsbBuses::_populated = false;
 
-usbtop::UsbBuses::~UsbBuses()
-{
-	list_buses_t::const_iterator it;
-	for (it = _buses.begin(); it != _buses.end(); it++) {
-		delete it->second;
-	}
-}
-
 void usbtop::UsbBuses::list(bus_func_t f, const char* filter)
 {
-	char errbuf[PCAP_ERRBUF_SIZE];
-	pcap_if_t* list_devs;
-	if (pcap_findalldevs(&list_devs, errbuf) < 0) {
-		std::cerr << "Unable to list available devices: " << errbuf << std::endl;
-		pcap_freealldevs(list_devs);
-		exit(1);
-	}
-
-	pcap_if_t* cur_dev = list_devs;
-	while (cur_dev) {
-		if (cur_dev->name) {
-			if ((strlen(cur_dev->name) > g_len_usb_dev_start) &&
-				(!filter || (filter && strcmp(cur_dev->name, filter) == 0)) &&
-			    (memcmp(cur_dev->name, USB_DEVICE_START, g_len_usb_dev_start) == 0)) {
-				size_t bus_id = atoll(&cur_dev->name[g_len_usb_dev_start+1]);
-				f(bus_id, cur_dev->name, cur_dev->description);
-			}
-		}
-		cur_dev = cur_dev->next;
-	}
-
-	pcap_freealldevs(list_devs);
+  char errbuf[PCAP_ERRBUF_SIZE];
+  pcap_if_t* list_devs;
+  if (pcap_findalldevs(&list_devs, errbuf) < 0) {
+    std::cerr << "Unable to list available devices: " << errbuf << std::endl;
+    pcap_freealldevs(list_devs);
+    exit(1);
+  }
+
+  pcap_if_t* cur_dev = list_devs;
+  while (cur_dev) {
+    if (cur_dev->name) {
+      if ((strlen(cur_dev->name) > g_len_usb_dev_start) &&
+          (!filter || (filter && strcmp(cur_dev->name, filter) == 0)) &&
+          (memcmp(cur_dev->name, USB_DEVICE_START, g_len_usb_dev_start) == 0)) {
+        size_t bus_id = atoll(&cur_dev->name[g_len_usb_dev_start]);
+        f(bus_id, cur_dev->name, cur_dev->description);
+      }
+    }
+    cur_dev = cur_dev->next;
+  }
+
+  pcap_freealldevs(list_devs);
 }
 
 void usbtop::UsbBuses::show(const char* filter)
 {
-	printf("Name\t\tDescription\n");
-	printf("---------------------------\n");
+  printf("Name\t\tDescription\n");
+  printf("---------------------------\n");
 	list([](UsbBus::id_type /*bus_id*/, const char* name, const char* desc)
-		{
-			printf("%s", name);
-			if (desc) {
-				printf("\t\t%s", desc);
-			}
-			printf("\n");
-		}, filter);
+    {
+      printf("%s", name);
+      if (desc) {
+        printf("\t\t%s", desc);
+      }
+      printf("\n");
+    }, filter);
 }
 
 void usbtop::UsbBuses::populate(const char* filter)
 {
-	if (_populated) {
-		return;
-	}
-
-	list([](UsbBus::id_type bus_id, const char* name, const char* desc)
-		{
-			add_bus(bus_id, name, desc);
-		}, filter);
-	_populated = true;
+  if (_populated) {
+    return;
+  }
+
+  list([](UsbBus::id_type bus_id, const char* name, const char* desc)
+    {
+      add_bus(bus_id, name, desc);
+    }, filter);
+  _populated = true;
 }
 
 void usbtop::UsbBuses::add_bus(UsbBus::id_type bus_id, const char* name, const char* desc)
 {
-	_buses.insert(std::make_pair(bus_id, new UsbBus(bus_id, name, desc)));
+  _buses.emplace(std::piecewise_construct,
+      std::forward_as_tuple(bus_id),
+      std::forward_as_tuple(bus_id, name, desc));
 }
 
 usbtop::UsbBus* usbtop::UsbBuses::get_bus(UsbBus::id_type bus_id)
 {
-	list_buses_t::iterator it = _buses.find(bus_id);
-	if (it == _buses.end()) {
-		return NULL;
-	}
+  list_buses_t::iterator it = _buses.find(bus_id);
+  if (it == _buses.end()) {
+    return NULL;
+  }
 
-	return it->second;
+  return &it->second;
 }
 
 size_t usbtop::UsbBuses::size()
 {
-	return _buses.size();
+  return _buses.size();
 }

src/main.cpp

@@ -37,6 +37,7 @@
 #include <cstdlib>
 #include <thread>
 #include <iostream>
+#include <functional>
 
 #include <pcap/pcap.h>
 

src/usb_stats.cpp

@@ -36,44 +36,44 @@
 // TODO: 
 //  * we need to do some real tests & math here to have accurate values.. !
 
-double usbtop::Stats::_t0 = 0;
+double usbtop::Stats::t0_ = 0;
 
 usbtop::Stats::Stats():
-	_nbytes(0),
-	_tN(0),
-	_nsamples(0),
-	_inst_data(LIVE_SAMPLE_COUNT),
-	_last_inst_bw(0.0),
-	_stats_window(1.0)
+	nbytes_(0),
+	tN_(0),
+	nsamples_(0),
+	inst_data_(LIVE_SAMPLE_COUNT),
+	last_inst_bw_(0.0),
+	stats_window_(1.0)
 {
 }
 
 void usbtop::Stats::init()
 {
-	_t0 = usbtop::tools::get_current_timestamp();
+	t0_ = usbtop::tools::get_current_timestamp();
 }
 
 void usbtop::Stats::push(double timestamp, size_t spacket)
 {
-	_nsamples++;
-	_nbytes += spacket;
+	nsamples_++;
+	nbytes_ += spacket;
 
-	double first_ts = timestamp-_stats_window;
+	double first_ts = timestamp-stats_window_;
 
 	// Remove oldest samples
 
-	_inst_data.push_back(sample_t(timestamp, spacket));
+	inst_data_.push_back(sample_t(timestamp, spacket));
 
-	if (timestamp < _tN+0.2) {
+	if (timestamp < tN_+0.2) {
 		return;
 	}
 
-	_tN = timestamp;
+	tN_ = timestamp;
 
 	boost::circular_buffer<sample_t>::iterator it;
 	boost::circular_buffer<sample_t>::iterator it_last_rem;
 	bool to_rem = false;
-	for (it = _inst_data.begin(); it != _inst_data.end(); it++) {
+	for (it = inst_data_.begin(); it != inst_data_.end(); it++) {
 		if (it->first >= first_ts) {
 			break;
 		}
@@ -82,33 +82,33 @@ void usbtop::Stats::push(double timestamp, size_t spacket)
 	}
 	if (to_rem) {
 		it_last_rem++;
-		_inst_data.erase(_inst_data.begin(), it_last_rem);
+		inst_data_.erase(inst_data_.begin(), it_last_rem);
 	}
 	// Compute instant bw at this instant
 	size_t tsize = 0.0;
 	{
 		boost::circular_buffer<sample_t>::const_iterator it;
-		for (it = _inst_data.begin(); it != _inst_data.end(); it++) {
+		for (it = inst_data_.begin(); it != inst_data_.end(); it++) {
 			tsize += it->second;
 		}
 	}
-	const double first_ts_buf = _inst_data.front().first;
+	const double first_ts_buf = inst_data_.front().first;
 	if (timestamp == first_ts_buf) {
-		_last_inst_bw = 0.0;
+		last_inst_bw_ = 0.0;
 	}
 	else {
-		_last_inst_bw = ((double)tsize)/(timestamp-first_ts_buf);
+		last_inst_bw_ = ((double)tsize)/(timestamp-first_ts_buf);
 	}
 }
 
 double usbtop::Stats::bw_instant() const
 {
 	double last_ts_packet, last_inst_bw;
-	last_ts_packet = _tN;
-	last_inst_bw = _last_inst_bw;
+	last_ts_packet = tN_;
+	last_inst_bw = last_inst_bw_;
 
 	double cur_ts = tools::get_current_timestamp();
-	if (cur_ts >= last_ts_packet+_stats_window) {
+	if (cur_ts >= last_ts_packet+stats_window_) {
 		// No packet in current window. Returns 0
 		return 0.0;
 	}
@@ -118,5 +118,5 @@ double usbtop::Stats::bw_instant() const
 
 double usbtop::Stats::bw_mean() const
 {
-	return (double)_nbytes/(_tN-_t0);
+	return (double)nbytes_/(tN_-t0_);
 }