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 47826e49431db082fc9d4ea93d6026a233e37bee..cefd11ec53cd7297525a1f239f1b8fd1ace6b431 100644 (file)
--- 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 5afbd077049c7ada43af147a48e0128514bc9b85..b58092302dcc84de9917fb149292e973094996de 100644 (file)
--- 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);
 
 /*