|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH 5/6] lib/devfs: Fix checkpath warnings
Hi Vlad,
Please see my comments inline.
On 4/17/19 2:32 PM, Vlad-Andrei BĂDOIU (78692) wrote:
> Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>
> ---
> lib/devfs/devfs_vnops.c | 57 +++++++++++++++++++++--------------------
> lib/devfs/device.c | 15 +++++------
> 2 files changed, 36 insertions(+), 36 deletions(-)
>
> diff --git a/lib/devfs/devfs_vnops.c b/lib/devfs/devfs_vnops.c
> index 925179f3..f62ce659 100644
> --- a/lib/devfs/devfs_vnops.c
> +++ b/lib/devfs/devfs_vnops.c
> @@ -62,7 +62,7 @@
> #include <uk/ctors.h>
>
> #ifdef DEBUG_DEVFS
> -#define DPRINTF(X) uk_pr_debug X
> +#define DPRINTF(X) (uk_pr_debug X)
Let's just get rid of the double parentheses throughout the code and
redefine this to uk_pr_debug(X).
> #else
> #define DPRINTF(X)
> #endif
> @@ -79,21 +79,21 @@ devfs_open(struct vfscore_file *fp)
> struct device *dev;
> int error;
>
> - DPRINTF(("devfs_open: path=%s\n", path));
> + DPRINTF(("%s: path=%s\n", __func__, path));
>
> if (!strcmp(path, "/")) /* root ? */
> return 0;
>
> if (vp->v_flags & VPROTDEV) {
> - DPRINTF(("devfs_open: failed to open protected device.\n"));
> - return EPERM;
> + DPRINTF(("%s: failed to open protected device.\n", __func__));
> + return -EPERM;
> }
> if (*path == '/')
> path++;
> error = device_open(path, fp->f_flags & DO_RWMASK, &dev);
> if (error) {
> - DPRINTF(("devfs_open: can not open device = %s error=%d\n",
> - path, error));
> + DPRINTF(("%s: can not open device = %s error=%d\n",
> + __func__, path, error));
> return error;
> }
> vp->v_data = (void *)dev; /* Store private data */
> @@ -104,24 +104,25 @@ static int
> devfs_close(struct vnode *vp, struct vfscore_file *fp)
> {
>
> - DPRINTF(("devfs_close: fp=%x\n", fp));
> + DPRINTF(("%s: fp=%x\n", __func__, fp));
>
> if (!strcmp(fp->f_dentry->d_path, "/")) /* root ? */
> return 0;
>
> - return device_close((struct device*)vp->v_data);
> + return device_close((struct device *)vp->v_data);
> }
>
> static int
> -devfs_read(struct vnode *vp, struct vfscore_file *fp, struct uio *uio, int
> ioflags)
> +devfs_read(struct vnode *vp, struct vfscore_file *fp,
> + struct uio *uio, int ioflags)
> {
> - return device_read((struct device*)vp->v_data, uio, ioflags);
> + return device_read((struct device *)vp->v_data, uio, ioflags);
> }
>
> static int
> devfs_write(struct vnode *vp, struct uio *uio, int ioflags)
> {
> - return device_write((struct device*)vp->v_data, uio, ioflags);
> + return device_write((struct device *)vp->v_data, uio, ioflags);
> }
>
> static int
> @@ -129,8 +130,8 @@ devfs_ioctl(struct vnode *vp, struct vfscore_file *fp,
> u_long cmd, void *arg)
> {
> int error;
>
> - error = device_ioctl((struct device*)vp->v_data, cmd, arg);
> - DPRINTF(("devfs_ioctl: cmd=%x\n", cmd));
> + error = device_ioctl((struct device *)vp->v_data, cmd, arg);
> + DPRINTF(("%s: cmd=%x\n", __func__, cmd));
> return error;
> }
>
> @@ -141,21 +142,21 @@ devfs_lookup(struct vnode *dvp, char *name, struct
> vnode **vpp)
> struct vnode *vp;
> int error, i;
>
> - DPRINTF(("devfs_lookup:%s\n", name));
> + DPRINTF(("%s:%s\n", __func__, name));
>
> *vpp = NULL;
>
> if (*name == '\0')
> - return ENOENT;
> + return -ENOENT;
>
> i = 0;
> error = 0;
> info.cookie = 0;
> for (;;) {
> error = device_info(&info);
> - if (error) {
> - return ENOENT;
> - }
> + if (error)
> + return -ENOENT;
> +
> if (!strncmp(info.name, name, MAXDEVNAME))
> break;
> i++;
> @@ -166,12 +167,12 @@ devfs_lookup(struct vnode *dvp, char *name, struct
> vnode **vpp)
> return 0;
> }
> if (!vp)
> - return ENOMEM;
> + return -ENOMEM;
> vp->v_type = (info.flags & D_CHR) ? VCHR : VBLK;
> if (info.flags & D_TTY)
> vp->v_flags |= VISTTY;
>
> - vp->v_mode = (mode_t)(S_IRUSR | S_IWUSR);
> + vp->v_mode = (mode_t)(0600);
Why don't you use macros anymore? Because of checkpatch? I think it
would be better to keep the macros, it makes the code more portable.
>
> *vpp = vp;
>
> @@ -187,7 +188,7 @@ devfs_readdir(struct vnode *vp, struct vfscore_file *fp,
> struct dirent *dir)
> struct devinfo info;
> int error, i;
>
> - DPRINTF(("devfs_readdir offset=%d\n", fp->f_offset));
> + DPRINTF(("%s: offset=%d\n", __func__, fp->f_offset));
>
> i = 0;
> error = 0;
> @@ -195,7 +196,7 @@ devfs_readdir(struct vnode *vp, struct vfscore_file *fp,
> struct dirent *dir)
> do {
> error = device_info(&info);
> if (error)
> - return ENOENT;
> + return -ENOENT;
> } while (i++ != fp->f_offset);
>
> dir->d_type = 0;
> @@ -207,7 +208,7 @@ devfs_readdir(struct vnode *vp, struct vfscore_file *fp,
> struct dirent *dir)
> dir->d_fileno = fp->f_offset;
> // dir->d_namlen = strlen(dir->d_name);
>
> - DPRINTF(("devfs_readdir: %s\n", dir->d_name));
> + DPRINTF(("%s: %s\n", __func__, dir->d_name));
> fp->f_offset++;
> return 0;
> }
> @@ -237,19 +238,19 @@ vop_nullop(void)
> int
> vop_einval(void)
> {
> - return EINVAL;
> + return -EINVAL;
> }
>
> int
> vop_eperm(void)
> {
> - return EPERM;
> + return -EPERM;
> }
>
> int
> vop_erofs(void)
> {
> - return EROFS;
> + return -EROFS;
> }
>
> int
> @@ -334,14 +335,14 @@ UK_FS_REGISTER(fs_devfs);
> __constructor_prio(101) static void devfs_init(void)
> {
> int ret;
> -
> +
> ret = mount("", "/", "ramfs", 0, NULL);
> if (ret != 0) {
> DPRINTF(("Failed to mount / in %s\n", __func__));
> return;
> }
>
> - ret = mkdir("/dev", S_IRWXU);
> + ret = mkdir("/dev", 0700);
Same about the macro flags.
> if (ret != 0) {
> DPRINTF(("Failed to mkdir /dev in %s\n", __func__));
> return;
> diff --git a/lib/devfs/device.c b/lib/devfs/device.c
> index 372e3cb1..8b2ae166 100644
> --- a/lib/devfs/device.c
> +++ b/lib/devfs/device.c
> @@ -56,7 +56,7 @@
> static struct uk_mutex devfs_lock = UK_MUTEX_INITIALIZER(devfs_lock);
>
> /* list head of the devices */
> -static struct device *device_list = NULL;
> +static struct device *device_list;
>
> /*
> * Look up a device object by device name.
> @@ -84,7 +84,7 @@ struct partition_table_entry {
> uint16_t ending_cylinder:10;
> uint32_t rela_sector;
> uint32_t total_sectors;
> -} __attribute__((packed));
> +} __packed;
>
>
> void device_register(struct device *dev, const char *name, int flags)
> @@ -190,7 +190,7 @@ device_reference(struct device *dev)
> uk_mutex_lock(&devfs_lock);
> if (!device_valid(dev)) {
> uk_mutex_unlock(&devfs_lock);
> - return ENODEV;
> + return -ENODEV;
> }
> dev->refcnt++;
> uk_mutex_unlock(&devfs_lock);
> @@ -232,7 +232,7 @@ device_destroy(struct device *dev)
> uk_mutex_lock(&devfs_lock);
> if (!device_valid(dev)) {
> uk_mutex_unlock(&devfs_lock);
> - return ENODEV;
> + return -ENODEV;
> }
> dev->active = 0;
> uk_mutex_unlock(&devfs_lock);
> @@ -260,14 +260,13 @@ device_open(const char *name, int mode, struct device
> **devp)
> uk_mutex_lock(&devfs_lock);
> if ((dev = device_lookup(name)) == NULL) {
> uk_mutex_unlock(&devfs_lock);
> - return ENXIO;
> + return -ENXIO;
> }
> uk_mutex_unlock(&devfs_lock);
>
> error = device_reference(dev);
> - if (error) {
> + if (error)
> return error;
> - }
>
> ops = dev->driver->devops;
> UK_ASSERT(ops->open != NULL);
> @@ -388,7 +387,7 @@ device_info(struct devinfo *info)
> int
> enodev(void)
> {
> - return ENODEV;
> + return -ENODEV;
> }
>
> int
>
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |