refactor: more code-style improvements

include/villas/graph/directed.hpp

@@ -175,9 +175,9 @@ public:
 				logger->debug("Remove edge {}", edgeId);
 				// remove edge from global edge list
 				it = edges.erase(it);
-			} else {
-				++it;
 			}
+			else
+				++it;
 		}
 
 		logger->debug("Remove vertex {}", vertexId);
@@ -200,10 +200,10 @@ public:
 	             Path &path,
 	             check_path_fn pathCheckFunc = checkPath)
 	{
-		if (fromVertexId == toVertexId) {
+		if (fromVertexId == toVertexId)
 			// arrived at the destination
 			return true;
-		} else {
+		else {
 			auto fromVertex = getVertex(fromVertexId);
 
 			for (auto &edgeId : fromVertex->edges) {
@@ -228,14 +228,12 @@ public:
 				path.push_back(edgeId);
 
 				// recursive, depth-first search
-				if (getPath(edgeOfFromVertex->to, toVertexId, path, pathCheckFunc) and
-				   pathCheckFunc(path)) {
+				if (getPath(edgeOfFromVertex->to, toVertexId, path, pathCheckFunc) and pathCheckFunc(path))
 					// path found, we're done
-				    return true;
-				} else {
+					return true;
+				else
 					// tear down path that didn't lead to the destination
 					path.pop_back();
-				}
 			}
 		}
 

include/villas/kernel/pci.hpp

@@ -83,21 +83,22 @@ struct Region {
 };
 
 class Device {
-
 public:
 	Device(Id i, Slot s) :
 		id(i),
 		slot(s)
 	{ }
 
-	bool
-	operator==(const Device &other);
+	Device(Id i) :
+		id(i)
+	{ }
 
-	bool
-	operator==(const Id &other);
+	Device(Slot s) :
+		slot(s)
+	{ }
 
 	bool
-	operator==(const Slot &other);
+	operator==(const Device &other);
 
 	/** Get currently loaded driver for device */
 	std::string
@@ -118,7 +119,7 @@ public:
 	Slot slot;
 };
 
-class DeviceList : public std::list<Device> {
+class DeviceList : public std::list<std::shared_ptr<Device>> {
 public:
 	/** Initialize Linux PCI handle.
 	 *
@@ -126,11 +127,14 @@ public:
 	 */
 	DeviceList();
 
-	Device &
-	lookupDevice(const Slot &slot);
+	DeviceList::value_type
+	lookupDevice(const Slot &s);
+
+	DeviceList::value_type
+	lookupDevice(const Id &i);
 
-	Device &
-	lookupDevice(const Id &id);
+	DeviceList::value_type
+	lookupDevice(const Device &f);
 };
 
 } /* namespace pci */

lib/kernel/pci.cpp

@@ -82,22 +82,34 @@ DeviceList::DeviceList()
 		if (ret != 4)
 			error("Failed to parse PCI slot number: %s", e->d_name);
 
-		emplace_back(id, slot);
+		emplace_back(std::make_shared<Device>(id, slot));
 	}
 
 	closedir(dp);
 }
 
-Device &
+DeviceList::value_type
 DeviceList::lookupDevice(const Slot &s)
 {
-	return *std::find(begin(), end(), s);
+	return *std::find_if(begin(), end(), [s](const DeviceList::value_type &d) {
+		return d->slot == s;
+	});
 }
 
-Device &
+DeviceList::value_type
 DeviceList::lookupDevice(const Id &i)
 {
-	return *std::find(begin(), end(), i);
+	return *std::find_if(begin(), end(), [i](const DeviceList::value_type &d) {
+		return d->id == i;
+	});
+}
+
+DeviceList::value_type
+DeviceList::lookupDevice(const Device &d)
+{
+	return *std::find_if(begin(), end(), [d](const DeviceList::value_type &e) {
+		return *e == d;
+	});
 }
 
 Id::Id(const std::string &str) :
@@ -259,18 +271,6 @@ Device::operator==(const Device &f)
 	return id == f.id && slot == f.slot;
 }
 
-bool
-Device::operator==(const Slot &s)
-{
-	return slot == s;
-}
-
-bool
-Device::operator==(const Id &i)
-{
-	return id == i;
-}
-
 std::list<Region>
 Device::getRegions() const
 {

lib/kernel/vfio.cpp

@@ -78,7 +78,6 @@ namespace villas {
 Container::Container()
     : iova_next(0)
 {
-
 	Logger logger = logging.get("kernel:vfio");
 
 	static constexpr const char* requiredKernelModules[] = {
@@ -86,49 +85,38 @@ Container::Container()
 	};
 
 	for (const char* module : requiredKernelModules) {
-		if (kernel::module_load(module) != 0) {
-			logger->error("Kernel module '{}' required but could not be loaded. "
+		if (kernel::module_load(module) != 0)
+			throw RuntimeError("Kernel module '{}' required but could not be loaded. "
 			              "Please load manually!", module);
-			throw std::exception();
-		}
 	}
 
 	/* Open VFIO API */
 	fd = open(VFIO_DEV, O_RDWR);
-	if (fd < 0) {
-		logger->error("Failed to open VFIO container");
-		throw std::exception();
-	}
+	if (fd < 0)
+		throw RuntimeError("Failed to open VFIO container");
 
 	/* Check VFIO API version */
 	version = ioctl(fd, VFIO_GET_API_VERSION);
-	if (version < 0 || version != VFIO_API_VERSION) {
-		logger->error("Failed to get VFIO version");
-		throw std::exception();
-	}
+	if (version < 0 || version != VFIO_API_VERSION)
+		throw RuntimeError("Failed to get VFIO version");
 
 	/* Check available VFIO extensions (IOMMU types) */
 	extensions = 0;
 	for (unsigned int i = VFIO_TYPE1_IOMMU; i <= VFIO_NOIOMMU_IOMMU; i++) {
 		int ret = ioctl(fd, VFIO_CHECK_EXTENSION, i);
-		if (ret < 0) {
-			logger->error("Failed to get VFIO extensions");
-			throw std::exception();
-		}
-		else if (ret > 0) {
+		if (ret < 0)
+			throw RuntimeError("Failed to get VFIO extensions");
+		else if (ret > 0)
 			extensions |= (1 << i);
-		}
 	}
 
 	hasIommu = false;
 
 	if (not (extensions & (1 << VFIO_NOIOMMU_IOMMU))) {
-		if (not (extensions & (1 << VFIO_TYPE1_IOMMU))) {
-			logger->error("No supported IOMMU extension found");
-			throw std::exception();
-		} else {
+		if (not (extensions & (1 << VFIO_TYPE1_IOMMU)))
+			throw RuntimeError("No supported IOMMU extension found");
+		else
 			hasIommu = true;
-		}
 	}
 
 	logger->debug("Version:    {:#x}", version);
@@ -148,9 +136,8 @@ Container::~Container()
 
 	/* Close container */
 	int ret = close(fd);
-	if (ret < 0) {
+	if (ret < 0)
 		logger->error("Cannot close vfio container fd {}", fd);
-	}
 }
 
 
@@ -158,6 +145,7 @@ std::shared_ptr<Container>
 Container::create()
 {
 	std::shared_ptr<Container> container { new Container };
+
 	return container;
 }
 
@@ -227,19 +215,15 @@ Container::attachDevice(const char* name, int index)
 
 	/* Open device fd */
 	device->fd = ioctl(group.fd, VFIO_GROUP_GET_DEVICE_FD, name);
-	if (device->fd < 0) {
-		logger->error("Failed to open VFIO device: {}", device->name);
-		throw std::exception();
-	}
+	if (device->fd < 0)
+		throw RuntimeError("Failed to open VFIO device: {}", device->name);
 
 	/* Get device info */
 	device->info.argsz = sizeof(device->info);
 
 	int ret = ioctl(device->fd, VFIO_DEVICE_GET_INFO, &device->info);
-	if (ret < 0) {
-		logger->error("Failed to get VFIO device info for: {}", device->name);
-		throw std::exception();
-	}
+	if (ret < 0)
+		throw RuntimeError("Failed to get VFIO device info for: {}", device->name);
 
 	logger->debug("Device has {} regions", device->info.num_regions);
 	logger->debug("Device has {} IRQs", device->info.num_irqs);
@@ -258,10 +242,8 @@ Container::attachDevice(const char* name, int index)
 		region.index = i;
 
 		ret = ioctl(device->fd, VFIO_DEVICE_GET_REGION_INFO, &region);
-		if (ret < 0) {
-			logger->error("Failed to get region of VFIO device: {}", device->name);
-			throw std::exception();
-		}
+		if (ret < 0)
+			throw RuntimeError("Failed to get region of VFIO device: {}", device->name);
 
 		device->regions[i] = region;
 	}
@@ -276,10 +258,8 @@ Container::attachDevice(const char* name, int index)
 		irq.index = i;
 
 		ret = ioctl(device->fd, VFIO_DEVICE_GET_IRQ_INFO, &irq);
-		if (ret < 0) {
-			logger->error("Failed to get IRQs of VFIO device: {}", device->name);
-			throw std::exception();
-		}
+		if (ret < 0)
+			throw RuntimeError("Failed to get IRQs of VFIO device: {}", device->name);
 
 		device->irqs[i] = irq;
 	}
@@ -319,8 +299,7 @@ Container::attachDevice(const pci::Device &pdev)
 					"(https://villas.fein-aachen.org/doc/fpga-setup.html) "
 					"for help with troubleshooting.");
 
-		logger->error("Failed to get IOMMU group of device");
-		throw std::exception();
+		throw RuntimeError("Failed to get IOMMU group of device");
 	}
 
 	/* VFIO device name consists of PCI BDF */
@@ -333,10 +312,8 @@ Container::attachDevice(const pci::Device &pdev)
 	device.pci_device = &pdev;
 
 	/* Check if this is really a vfio-pci device */
-	if (not device.isVfioPciDevice()) {
-		logger->error("Device is not a vfio-pci device");
-		throw std::exception();
-	}
+	if (not device.isVfioPciDevice())
+		throw RuntimeError("Device is not a vfio-pci device");
 
 	return device;
 }
@@ -434,12 +411,10 @@ Container::getOrAttachGroup(int index)
 
 	/* Group not yet part of this container, so acquire ownership */
 	auto group = Group::attach(*this, index);
-	if (not group) {
-		logger->error("Failed to attach to IOMMU group: {}", index);
-		throw std::exception();
-	} else {
+	if (not group)
+		throw RuntimeError("Failed to attach to IOMMU group: {}", index);
+	else
 		logger->debug("Attached new group {} to VFIO container", index);
-	}
 
 	/* Push to our list */
 	groups.push_back(std::move(group));
@@ -764,18 +739,16 @@ Group::~Group()
 	/* Release memory and close fds */
 	devices.clear();
 
-	if (fd < 0) {
+	if (fd < 0)
 		logger->debug("Destructing group that has not been attached");
-	} else {
+	else {
 		int ret = ioctl(fd, VFIO_GROUP_UNSET_CONTAINER);
-		if (ret != 0) {
+		if (ret != 0)
 			logger->error("Cannot unset container for group fd {}", fd);
-		}
 
 		ret = close(fd);
-		if (ret != 0) {
+		if (ret != 0)
 			logger->error("Cannot close group fd {}", fd);
-		}
 	}
 }
 

lib/memory.cpp

@@ -210,16 +210,15 @@ HostDmaRam::HostDmaRamAllocator::HostDmaRamAllocator(int num) :
 		void* buf = mmap(nullptr, getSize(), PROT_READ|PROT_WRITE, MAP_SHARED, bufFd, 0);
 		close(bufFd);
 
-		if (buf != MAP_FAILED) {
+		if (buf != MAP_FAILED)
 			mm.createMapping(reinterpret_cast<uintptr_t>(buf), 0, getSize(),
 			                 getName() + "-VA",
 			                 mm.getProcessAddressSpace(), getAddrSpaceId());
-		} else {
+		else
 			logger->warn("Cannot map {}", bufPath);
-		}
-	} else {
-		logger->warn("Cannot open {}", bufPath);
 	}
+	else
+		logger->warn("Cannot open {}", bufPath);
 
 	logger->info("Mapped {} of size {} bytes", bufPath, getSize());
 }

lib/memory_manager.cpp

@@ -237,18 +237,17 @@ MemoryTranslation::operator+=(const MemoryTranslation &other)
 	/* The source stays the same and can only increase with merged translations */
 	//this->src = this->src;
 
-	if (otherSrcIsSmaller) {
+	if (otherSrcIsSmaller)
 		/* Other mapping starts at lower addresses, so we actually arrive at
 		 * higher addresses
 		 */
 		this->dst += diff_lo;
-	} else {
+	else
 		/* Other mapping starts at higher addresses than this, so we have to
 		 * increase the start
 		 * NOTE: for addresses equality, this just adds 0
 		 */
 		this->src += diff_lo;
-	}
 
 	logger->debug("result src:     {:#x}", this->src);
 	logger->debug("result dst:     {:#x}", this->dst);