Re: seq_file API strangeness

[Tigran Aivazian]

Ah, I know why I thought they took different arguments! Because I was 
creating two files --- binary and ascii versions and they shared the same 
"open_common" routine which didn't need an 'inode' so I was only passing 
'file' and appropriate 'seq_operations' pointer to it :)

On Sat, 15 Nov 2003, Tigran Aivazian wrote:

> Hi Al,
> 
> Yes, you are right, thank you. I don't know why I thought open/release 
> took different arguments. Yes, calling seq_release(inode,file) is the 
> right way, I will change my code.
> 
> Kind regards
> Tigran
> 
> On Fri, 14 Nov 2003 viro@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx wrote:
> 
> > On Fri, Nov 14, 2003 at 08:55:48PM +0000, Tigran Aivazian wrote:
> > > In the ->open() method I allocate a seq->private like this:
> > > 
> > >   err = seq_open(file, sop);
> > >   if (!err) {
> > > 	struct seq_file *m = file->private_data;
> > > 
> > > 	m->private = kmalloc(sizeof(struct ctask), GFP_KERNEL);
> > >         if (!m->private) {
> > >                         kfree(file->private_data);
> > >                         return -ENOMEM;
> > >         }
> > >   }
> > > 
> > > Now, freeing the structure that I did not allocate (file->private_data 
> > > allocated in seq_open()) is not nice. But calling seq_release() from 
> > > ->open() method is not nice either (different arguments, namely 'inode'
> > 
> > I beg your pardon?  What different arguments?
> > 
> > ->open() gets struct inode * and struct file *
> > ->release() gets exactly the same.
> > seq_release() is what you use as ->release()
> > 
> > What's the problem?
> > 
> > > and also m->buf is NULL at that point, although I believe kfree(NULL) is 
> > > not illegal).
> > 
> > Of course it is not illegal.  Moreover, if you just do open() immediately
> > followed by close(), you won't get non-NULL ->buf at all.  It's a perfectly
> > normal situation and seq_release() can handle it - no problems with that.
> > 
> > > What do you think?
> > 
> > 	if (!m->private) {
> > 		seq_release(inode, file);
> > 		return -ENOMEM;
> > 	}
> > 
> > Same as e.g. fs/proc/base.c does in similar situation (see mounts_open()).
> > 
> 
> 

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