lkml.org 
[lkml]   [2007]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [patch 1/13] signal/timer/event fds v6 - anonymous inode source ...
Date
On Friday 16 March 2007 01:22:15 Davide Libenzi wrote:

> +
> +static int ainofs_delete_dentry(struct dentry *dentry);
> +static struct inode *aino_getinode(void);
> +static struct inode *aino_mkinode(void);
> +static int ainofs_get_sb(struct file_system_type *fs_type, int flags,
> + const char *dev_name, void *data, struct vfsmount *mnt);
> +

In general, it would be good if you could just reorder your functions
so that you don't need any forward declarations like these. It makes
reviewing from bottom to top a little easier and it becomes obvious
that there are no recursions in the code.

> +static struct vfsmount *aino_mnt __read_mostly;
> +static struct inode *aino_inode;
> +static struct file_operations aino_fops = { };

Iirc, file_operations should be const.

> +int aino_getfd(int *pfd, struct inode **pinode, struct file **pfile,
> + char const *name, const struct file_operations *fops, void *priv)
> +{

Since this is meant to be a generic interface that can be used
from other subsystems, a kerneldoc style comment would be nice

> +static int __init aino_init(void)
> +{
> +
> + if (register_filesystem(&aino_fs_type))
> + goto epanic;
> +
> + aino_mnt = kern_mount(&aino_fs_type);
> + if (IS_ERR(aino_mnt))
> + goto epanic;
> +
> + aino_inode = aino_mkinode();
> + if (IS_ERR(aino_inode))
> + goto epanic;
> +
> + return 0;
> +
> +epanic:
> + panic("aino_init() failed\n");
> +}

panic() is a little harsh from a loadable module. If you mean
the aino support to be used as a module, this should probably
just return an error.

> +static void __exit aino_exit(void)
> +{
> + iput(aino_inode);
> + unregister_filesystem(&aino_fs_type);
> + mntput(aino_mnt);
> +}

but since the Makefile always has it as built-in, maybe you should
instead just kill the exit function and use fs_initcall instead
of init_module().

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

\
 
 \ /
  Last update: 2007-03-17 22:01    [from the cache]
©2003-2011 Jasper Spaans