linux-kernel - Re: [patch 1/13] signal/timer/event fds v8

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20070330160259.6e03705b.rdunlap@xenotime.net>
Date:	Fri, 30 Mar 2007 16:02:59 -0700
From:	Randy Dunlap <rdunlap@...otime.net>
To:	Davide Libenzi <davidel@...ilserver.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Oleg Nesterov <oleg@...sign.ru>
Subject: Re: [patch 1/13] signal/timer/event fds v8 - anonymous inode source
 ...

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 ***
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/