lkml.org 
[lkml]   [2005]   [Jan]   [6]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
/
DateThu, 6 Jan 2005 16:41:16 +0200
From"Michael S. Tsirkin" <>
SubjectRe: [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/

\
 
 \ /
  Last update: 2005-03-22 14:09    [from the cache]
©2003-2008