On Sat, Aug 04, 2012 at 11:08:24AM +0200, Hans de Goede wrote: > Why on earth are you putting that if block there and not simply putting > the command in the switch-case where it belongs? If you were afraid watchdog_ioctl_op > could cause issues, then maybe you should have studied the code? > > That would have thought you that it will always return -ENOIOCTLCMD for commands it > does not know how to handle, so it won't interfere with adding new standard commands. > > Adding this if block there is completely unnecessary and quite ugly IMHO. It is ugly, but it's also necessary for a couple reasons that you may have missed at first glance. First, this ioctl is addressing behavior that exists in the common framework layer, not in individual drivers, so it doesn't make too much sense to pass down to the drivers. That being said, we could do so if ENOIOCTLCMD worked as you suggest, but it doesn't quite. Drivers with an ioctl op will return ENOTTY, in which case the switch statement would work, but drivers with no ioctl op (e.g. softdog) cause watchdog_ioctl_op to return ENOIOCTLCMD, causing the switch statement to be skipped. In the end, we need WDIOC_NOCLOSEPING to be processed by the core, independent of what drivers do, since as I explained this is targeting the behavior of the core framework, not the drivers. For those reasons, I placed the code where it is, but I would be satisfied with a different approach that also works. Dave -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
