From: Marc Rittinghaus <marc.rittinghaus@unikraft.io>
Date: Thu, 20 Apr 2023 14:59:25 +0000 (+0200)
Subject: lib/devfs: Merge device_register and create
X-Git-Tag: RELEASE-0.13.0~106
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=d30efab0d0ef6fd88e5a949be1ed4cd342df0479;p=unikraft%2Funikraft.git

lib/devfs: Merge device_register and create

This commit merges the device_register() and device_create() functions
to safe a malloc() operation. The function is also changed to return
a proper error code instead of crashing. Furthermore, users of the
function are expected to provide a handler function for all device
operations. The early check via asserts ensures that missing handlers
are easily detected in debug builds without having to explicitly call
all device operations.

Checkpatch-Ignore: USE_NEGATIVE_ERRNO
Signed-off-by: Marc Rittinghaus <marc.rittinghaus@unikraft.io>
Reviewed-by: Razvan Deaconescu <razvand@unikraft.io>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #855
---

diff --git a/lib/devfs/device.c b/lib/devfs/device.c
index 47826e494..cefd11ec5 100644
--- a/lib/devfs/device.c
+++ b/lib/devfs/device.c
@@ -78,45 +78,6 @@ device_lookup(const char *name)
 	return NULL;
 }
 
-void device_register(struct device *dev, const char *name, int flags)
-{
-	size_t len;
-	void *priv = NULL;
-
-	UK_ASSERT(dev->driver != NULL);
-
-	/* Check the length of name. */
-	len = strnlen(name, MAXDEVNAME);
-	if (len == 0 || len >= MAXDEVNAME)
-		return;
-
-	uk_mutex_lock(&devfs_lock);
-
-	/* Check if specified name is already used. */
-	if (device_lookup(name) != NULL)
-		UK_CRASH("duplicate device");
-
-	/*
-	 * Allocate a device and device private data.
-	 */
-	if (dev->driver->devsz != 0) {
-		if ((priv = malloc(dev->driver->devsz)) == NULL)
-			UK_CRASH("devsz");
-		memset(priv, 0, dev->driver->devsz);
-	}
-
-	strlcpy(dev->name, name, len + 1);
-	dev->flags = flags;
-	dev->active = 1;
-	dev->refcnt = 1;
-	dev->private_data = priv;
-	dev->next = device_list;
-	device_list = dev;
-
-	uk_mutex_unlock(&devfs_lock);
-}
-
-
 /*
  * device_create - create new device object.
  *
@@ -124,33 +85,68 @@ void device_register(struct device *dev, const char *name, int flags)
  * I/O services to applications.  Returns device ID on
  * success, or 0 on failure.
  */
-struct device *
-device_create(struct driver *drv, const char *name, int flags)
+int
+device_create(struct driver *drv, const char *name, int flags,
+	      struct device **devp)
 {
 	struct device *dev;
 	size_t len;
 
-	UK_ASSERT(drv != NULL);
+	UK_ASSERT(drv);
+
+	UK_ASSERT(drv->devops->open);
+	UK_ASSERT(drv->devops->close);
+	UK_ASSERT(drv->devops->read);
+	UK_ASSERT(drv->devops->write);
+	UK_ASSERT(drv->devops->ioctl);
 
-	/* Check the length of name. */
+	/* Check the length of the name */
 	len = strnlen(name, MAXDEVNAME);
-	if (len == 0 || len >= MAXDEVNAME)
-		return NULL;
+	if (unlikely(len == 0 || len >= MAXDEVNAME))
+		return EINVAL;
+
+	/* Allocate a device and device memory if needed */
+	dev = malloc(sizeof(*dev) + drv->devsz);
+	if (unlikely(!dev))
+		return ENOMEM;
+
+	/* Allocate device private data if needed */
+	if (drv->devsz != 0) {
+		dev->private_data = (void *)(dev + 1);
+		memset(dev->private_data, 0, drv->devsz);
+	} else {
+		dev->private_data = NULL;
+	}
 
-	/*
-	 * Allocate a device structure.
+	memcpy(dev->name, name, len + 1);
+	dev->driver = drv;
+	dev->flags = flags;
+	dev->active = 1;
+	dev->refcnt = 1;
+
+	/* Insert device into device list. This must be done while holding
+	 * the devfs lock. We optimize for success and reduce lock holder time
+	 * by doing the check for name collision only here, instead of keeping
+	 * the lock held the whole time.
 	 */
-	dev = malloc(sizeof(struct device));
-	if (!dev) {
-		uk_pr_err("Failed to allocate device memory, creation failed\n");
-		return NULL;
+	uk_mutex_lock(&devfs_lock);
+
+	if (unlikely(device_lookup(name) != NULL)) {
+		uk_mutex_unlock(&devfs_lock);
+		free(dev);
+
+		return EEXIST;
 	}
 
-	dev->driver = drv;
-	device_register(dev, name, flags);
-	return dev;
-}
+	dev->next   = device_list;
+	device_list = dev;
+
+	uk_mutex_unlock(&devfs_lock);
 
+	if (devp)
+		*devp = dev;
+	return 0;
+}
 
 /*
  * Return true if specified device is valid.
diff --git a/lib/devfs/include/devfs/device.h b/lib/devfs/include/devfs/device.h
index 5afbd0770..b58092302 100644
--- a/lib/devfs/include/devfs/device.h
+++ b/lib/devfs/include/devfs/device.h
@@ -114,7 +114,8 @@ int device_info(struct devinfo *info);
 int devop_noop();
 int devop_eperm();
 
-struct device *device_create(struct driver *drv, const char *name, int flags);
+int device_create(struct driver *drv, const char *name, int flags,
+		  struct device **devp);
 int device_destroy(struct device *dev);
 
 /*