Re: [RFC][2.5] generic_usercopy() function (resend, forgot the patches)

Christoph Hellwig (hch@infradead.org)
Fri, 23 May 2003 11:19:41 +0100


On Fri, May 23, 2003 at 12:10:57PM +0200, Michael Hunold wrote:
> > Also file/inode should go away from the prototype (and the callback).
> > Only file is needed because inode == file->f_dentry->d_inode, and even
> > that one should be just some void *data instead.
>
> I only copied the function from videodev.c and renamed it, so please
> don't blame me for the way stuff is done there. 8-)
>
> Perhaps Gerd Knorr or Alan Cox can comment on your changes -- I'll have
> to investigate if all of these arguments are used anyway.

I don't blame you. I just think it shouldn't be added to the kernel API
without fixing it first :)

>
> >>+ case _IOC_READ: /* some v4l ioctls are marked wrong ... */
> > That's crap. Please move this workaround to v4l not into generic code.
>
> Is it possible to fix the definitons of the v4l ioctls instead without
> breaking binary compatiblity?

I doubt it. This just means v4l has to work around it's bug in the
v4l code, not in common code.

> >>+ case _IOC_WRITE:
> >>+ case (_IOC_WRITE | _IOC_READ):
> >>+ if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
> >>+ parg = sbuf;
> >>+ } else {
> >>+ /* too big to allocate from stack */
> >>+ mbuf = kmalloc(_IOC_SIZE(cmd),GFP_KERNEL);
> >>+ if (NULL == mbuf)
> >>+ return -ENOMEM;
> >>+ parg = mbuf;
> >
> >
> > I wonder whether you should just kmalloc always.
>
> Since this function is always used in user context and the memory is
> always freed afterwards, it should be possible to use vmalloc() or a
> static buffer (what's the maximum size?) instead.

Sorry, I meant it's probably not worth using the stack at all.
vmalloc() for ioctl buffers sounds like a very bad idea.

-
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/