Messages in this thread | | | Date | Thu, 6 Jan 2005 16:41:16 +0200 | From | "Michael S. Tsirkin" <> | Subject | Re: [PATCH] deprecate (un)register_ioctl32_conversion |
| |
Hello! Quoting r. Andrew Morton (akpm@osdl.org) "Re: [PATCH] deprecate (un)register_ioctl32_conversion": > > Christoph, I know you want to remove the inode parameter :) > > > > Otherwise I think -mm1 has the final version of the replacement. > > I merged Christoph's verion of the patch into -mm.
I see some more problems with this decision.
> > > asmlinkage long compat_sys_ioctl(unsigned int fd, unsigned int cmd, > unsigned long arg) > { > - struct file * filp; > + struct file *filp; > int error = -EBADF; > struct ioctl_trans *t; > > filp = fget(fd); > - if(!filp) > - goto out2; > - > - if (!filp->f_op || !filp->f_op->ioctl) { > - error = sys_ioctl (fd, cmd, arg); > + if (!filp) > goto out; > + > + if (!filp->f_op) { > + if (!filp->f_op->ioctl) > + goto do_ioctl; > + } else if (filp->f_op->compat_ioctl) { > + error = filp->f_op->compat_ioctl(filp, cmd, arg); > + goto out_fput; > }
Stare at this as I might, I dont understand why does it make sence. So if filp->f_op is NULL, you are then checking filp->f_op->ioctl? Looks like an oops to me.
What should be there:
> + if (!filp->f_op) { > + goto do_ioctl; > + } else if (filp->f_op->compat_ioctl) { > + error = filp->f_op->compat_ioctl(filp, cmd, arg); > + goto out_fput;
Look, was this patch even tested?
MST - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |