Hi, On 05/10/2012 10:10 AM, Hans de Goede wrote: <snip>
@@ -526,19 +518,28 @@ static long __video_do_ioctl(struct file *file, return ret; } - if ((vfd->debug& V4L2_DEBUG_IOCTL)&& - !(vfd->debug& V4L2_DEBUG_IOCTL_ARG)) { - v4l_print_ioctl(vfd->name, cmd); - printk(KERN_CONT "\n"); - } - if (test_bit(V4L2_FL_USES_V4L2_FH,&vfd->flags)) { vfh = file->private_data; use_fh_prio = test_bit(V4L2_FL_USE_FH_PRIO,&vfd->flags); + if (use_fh_prio) + ret_prio = v4l2_prio_check(vfd->prio, vfh->prio); } - if (use_fh_prio) - ret_prio = v4l2_prio_check(vfd->prio, vfh->prio); + if (v4l2_is_valid_ioctl(cmd)) {I would prefer for this check to be the first check in the function in the form of: if (!v4l2_is_valid_ioctl(cmd)) return -ENOTTY;
Oops, sorry we cannot do that because the drv my have its own custom ioctls handled by the default case <sigh>, so scrap this.
This will drop an indentation level from the code below and also drop an indentation level from the prio check introduced in a later patch, making the end result much more readable IMHO.+ struct v4l2_ioctl_info *info =&v4l2_ioctls[_IOC_NR(cmd)]; + + if (!test_bit(_IOC_NR(cmd), vfd->valid_ioctls)) { + if (!(info->flags& INFO_FL_CTRL) || + !(vfh&& vfh->ctrl_handler)) + return -ENOTTY;> + } > + }
I still think the above can be simplified a bit, in the form of: if (!test_bit(_IOC_NR(cmd), vfd->valid_ioctls) && !((info->flags & INFO_FL_CTRL) && vfh && vfh->ctrl_handler)) return -ENOTYY; Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
