Re: [PATCH v2 5/5] loop: Add LOOP_SET_FD_AND_STATUS ioctl

From: Christoph Hellwig
Date: Wed Apr 22 2020 - 13:44:37 EST


> + case LOOP_SET_FD: {
> + /* legacy case - pass in a zeroed out loop_info64, which
> + * corresponds with the default parameters we'd have used
> + * otherwise.
> + */

Nitpick: kernel coding style always has the /* on a line of its own.
Also please capitalize the first word in a multi-line comment.

> + struct loop_info64 info;
> +
> + memset(&info, 0, sizeof(info));
> + return loop_set_fd_and_status(lo, mode, bdev, arg, &info);
> + }
> + case LOOP_SET_FD_AND_STATUS: {
> + struct loop_fd_and_status fds;
> +
> + if (copy_from_user(&fds, argp, sizeof(fds)))
> + return -EFAULT;
> +
> + return loop_set_fd_and_status(lo, mode, bdev, fds.fd,
> + &fds.info);

What about actually passing the whole loop_fd_and_status structure?

Otherwise this looks good:

Reviewed-by: Christoph Hellwig <hch@xxxxxx>