Re: [patch 1/13] signal/timer/event fds v8 - anonymous inode source ...

[Randy Dunlap <rdunlap@xenotime.net>]

On Fri, 30 Mar 2007 15:44:15 -0700 (PDT) Davide Libenzi wrote:

> On Fri, 30 Mar 2007, Andrew Morton wrote:
> 
> > > +#include <asm/uaccess.h>
> > > +
> > > +
> > > +
> > 
> > Too many blank lines
> 
> It'd be interesting to know how much is enough. You use one, ppl says it 
> is too dense. You use more, ppl says it's too much.
> There's the one-line rule for inter-function spacing, but what's the 
> include-functions ones? Or the functions-data ones?

1 :)


> > > +static int ainofs_delete_dentry(struct dentry *dentry);
> > > +static struct inode *aino_mkinode(void);
> > 
> > Unneeded forward declaration.
> 
> Same here. You're the third says this, so I'm gonna change it. But pls 
> consider adding it to the coding style.
> 
> 
> 
> > > +static struct file_system_type aino_fs_type = {
> > > +	.name		= "ainofs",
> > > +	.get_sb		= ainofs_get_sb,
> > > +	.kill_sb	= kill_anon_super,
> > > +};
> > > +static struct dentry_operations ainofs_dentry_operations = {
> > > +	.d_delete	= ainofs_delete_dentry,
> > > +};
> > 
> > If this is moved elsewhere we can perhaps remove some or all of the
> > unpleasing static function forward-declarations.
> 
> Grrr :)

you are a puttycat


> > > +/**
> > > + * aino_getfd - creates a new file instance by hooking it up to and anonymous
> > > + *              inode, and a dentry that describe the "class" of the file
> > > + * @pfd:     [out]   pointer to the file descriptor
> > > + * @dpinode: [out]   pointer to the inode
> > > + * @pfile:   [out]   pointer to the file struct
> > > + * @name:    [in]    name of the "class" of the new file
> > > + * @fops     [in]    file operations for the new file
> > > + * @priv     [in]    private data for the new file (will be file's private_data)
> > 
> > The [in] and [out] thing is nice - does kerneldoc handle it appropriately?
> 
> No idea. It should come out as text at least.

Yes, it's just [nice] text.

But the function description needs to fit on one line.  If that's
not enough, put more description after the @params lines, separated
by a
 *
"blank" line.


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***