Re: [PATCH] deprecate (un)register_ioctl32_conversion

[Michael S. Tsirkin]

Hello!
Quoting r. Andrew Morton (akpm@xxxxxxxx) "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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/