Messages in this thread | | | Date | Fri, 23 May 2003 11:19:41 +0100 | From | Christoph Hellwig <> | Subject | Re: [RFC][2.5] generic_usercopy() function (resend, forgot the patches) |
| |
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/
| |