Messages in this thread | | | Date | Tue, 12 Mar 2013 13:06:14 +0000 | From | Al Viro <> | Subject | Re: pipe_release oops. |
| |
On Mon, Mar 11, 2013 at 06:05:43PM +0000, Al Viro wrote: > On Mon, Mar 11, 2013 at 08:10:10AM -0700, Linus Torvalds wrote: > > On Sun, Mar 10, 2013 at 5:35 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: > > > > > > Hmm... How the devil would things like pipe_read_open() get called, anyway? > > > pipe_rdwr_open() can be called, all right - that happens if you do pipe() > > > and then open() via /proc/self/fd/<n>. But how could pipe_read_open() and > > > pipe_write_open() be called? They are accessible only as ->open() of > > > read_pipefifo_fops/write_pipefifo_fops. And those are only used by > > > fifo_open(), which does reassign file->f_op to them, but does *not* call > > > their ->open()... > > > > .. same deal? Open the resulting fd from /proc/self/fd/<n> afterwards, > > which will call file->f_op->open(), no? > > Not really - that would call inode->i_fop, not file->f_op. You get dentry > of opened file, but file->f_op is set from scratch - not copied from the > original struct file.
While we are at it, I don't see any reason for having separate file_operations for r/o, w/o and r/w cases; the only differences are in EBADF-returning ->read() and ->write() (and ->f_mode checks in vfs_read() et.al. take care of that) and micro-optimizations in ->release() and ->fasync().
Frankly, I really wonder if we should simply use def_fifo_fops for ->i_fops in get_pipe_inode() and let open() via /proc/<pid>/fd/<n> act as it would for FIFOs, O_NONBLOCK and all. IOW, how about we simply merge all those file_operations in one, folding fifo.c into pipe.c? And to hell with any reassignments of ->f_op.
I'm probably missing something subtle here...
| |