Re: [PATCH] docuement filesystem helpers for custom 'struct file's
On Thu, Sep 20, 2007 at 10:25:56AM -0700, Dave Hansen wrote: > Should we do comments for every single function argument, or is it OK to > leave them out for the obvious ones? Yes, we should keep the kerneldoc comments coherent, even if it seems useless in a few cases. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] docuement filesystem helpers for custom 'struct file's
On Wed, 2007-09-19 at 15:07 -0700, Randy Dunlap wrote: > > They aren't quite in kernel-doc format. Holler if you need help > with that, or see examples, or > Documentation/kernel-doc-nano-HOWTO.txt. Should we do comments for every single function argument, or is it OK to leave them out for the obvious ones? init_file() gets really boring: > + * init_file - initialize a 'struct file' > + * @file: the already allocated 'struct file' to initialized > + * @mnt: the vfsmount on which the file resides > + * @dentry: the dentry for the file > + * @mode: the file's mode > + * @fop: the file's operations Seems like a waste of space to me, but I'd be happy to do them if that's the convention. -- Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] docuement filesystem helpers for custom 'struct file's
On Wed, 2007-09-19 at 15:07 -0700, Randy Dunlap wrote: They aren't quite in kernel-doc format. Holler if you need help with that, or see examples, or Documentation/kernel-doc-nano-HOWTO.txt. Should we do comments for every single function argument, or is it OK to leave them out for the obvious ones? init_file() gets really boring: + * init_file - initialize a 'struct file' + * @file: the already allocated 'struct file' to initialized + * @mnt: the vfsmount on which the file resides + * @dentry: the dentry for the file + * @mode: the file's mode + * @fop: the file's operations Seems like a waste of space to me, but I'd be happy to do them if that's the convention. -- Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] docuement filesystem helpers for custom 'struct file's
On Thu, Sep 20, 2007 at 10:25:56AM -0700, Dave Hansen wrote: Should we do comments for every single function argument, or is it OK to leave them out for the obvious ones? Yes, we should keep the kerneldoc comments coherent, even if it seems useless in a few cases. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] docuement filesystem helpers for custom 'struct file's
On Wed, 19 Sep 2007 10:47:23 -0700 Dave Hansen wrote:
> On Wed, 2007-09-19 at 18:26 +0100, Christoph Hellwig wrote:
> > On Mon, Sep 17, 2007 at 11:27:18AM -0700, Dave Hansen wrote:
> > >
> > > Christoph H. says this stands on its own and can go in before the
> > > rest of the r/o bind mount set.
> > >
> > > ---
> > >
> > > Some filesystems forego the vfs and may_open() and create their
> > > own 'struct file's.
> > >
> > > This patch creates a couple of helper functions which can be
> > > used by these filesystems, and will provide a unified place
> > > which the r/o bind mount code may patch.
> > >
> > > Also, rename an existing, static-scope init_file() to a less
> > > generic name.
> >
> > Looks good. But please provide a patch ontop of your patchkit to
> > add kernel-doc comments for the two new exported functions.
>
> Appended. I started to write the comments to describe the function
> arguments, but they all looked pretty retarded.
Hi Dave,
They aren't quite in kernel-doc format. Holler if you need help
with that, or see examples, or
Documentation/kernel-doc-nano-HOWTO.txt.
> > Are there any direct caller of get_empty_filp left after this patch?
> > We really should get rid of that export to stop people from shooting
> > themselves in the foot.
>
> Yeah, there are a few left. But, they're in the middle of what is
> sometimes tricky error handling, so I think we should trickle those in
> later.
>
>
> -- Dave
>
> lxc-dave/fs/file_table.c | 22 +-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff -puN fs/file_table.c~document fs/file_table.c
> --- lxc/fs/file_table.c~document 2007-09-19 10:31:13.0 -0700
> +++ lxc-dave/fs/file_table.c 2007-09-19 10:39:34.0 -0700
> @@ -137,6 +137,18 @@ fail:
>
> EXPORT_SYMBOL(get_empty_filp);
>
> +/**
> + * alloc_file - allocate and initialize a 'struct file' with
> + * the given arguments
* function_name - short description (on one line)
* @param1: descr1
* @param2: descr2
etc.
> + *
> + * Use this instead of get_empty_filp() to get a new
> + * 'struct file'. Do so because of the same initialization
> + * pitfalls reasons listed for init_file(). This is a
> + * preferred interface to using init_file().
> + *
> + * If all the callers of init_file() are eliminated, its
> + * code should be moved into this function.
> + */
> struct file *alloc_file(struct vfsmount *mnt, struct dentry *dentry,
> mode_t mode, const struct file_operations *fop)
> {
> @@ -152,7 +164,15 @@ struct file *alloc_file(struct vfsmount
> }
> EXPORT_SYMBOL(alloc_file);
>
> -/*
> +/**
> + * init_file - initialize a 'struct file'
> + * @file: the already allocated 'struct file' to initialized
> + * @mnt: the vfsmount on which the file resides
> + *
> + * Use this instead of setting the members directly. Doing so
> + * avoids making mistakes like forgetting the mntget() or
> + * forgetting to take a write on the mnt.
> + *
> * Note: This is a crappy interface. It is here to make
> * merging with the existing users of get_empty_filp()
> * who have complex failure logic easier. All users
OK, that one looks better. :)
---
~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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] docuement filesystem helpers for custom 'struct file's
On Wed, 19 Sep 2007 10:47:23 -0700 Dave Hansen wrote:
On Wed, 2007-09-19 at 18:26 +0100, Christoph Hellwig wrote:
On Mon, Sep 17, 2007 at 11:27:18AM -0700, Dave Hansen wrote:
Christoph H. says this stands on its own and can go in before the
rest of the r/o bind mount set.
---
Some filesystems forego the vfs and may_open() and create their
own 'struct file's.
This patch creates a couple of helper functions which can be
used by these filesystems, and will provide a unified place
which the r/o bind mount code may patch.
Also, rename an existing, static-scope init_file() to a less
generic name.
Looks good. But please provide a patch ontop of your patchkit to
add kernel-doc comments for the two new exported functions.
Appended. I started to write the comments to describe the function
arguments, but they all looked pretty retarded.
Hi Dave,
They aren't quite in kernel-doc format. Holler if you need help
with that, or see examples, or
Documentation/kernel-doc-nano-HOWTO.txt.
Are there any direct caller of get_empty_filp left after this patch?
We really should get rid of that export to stop people from shooting
themselves in the foot.
Yeah, there are a few left. But, they're in the middle of what is
sometimes tricky error handling, so I think we should trickle those in
later.
-- Dave
lxc-dave/fs/file_table.c | 22 +-
1 file changed, 21 insertions(+), 1 deletion(-)
diff -puN fs/file_table.c~document fs/file_table.c
--- lxc/fs/file_table.c~document 2007-09-19 10:31:13.0 -0700
+++ lxc-dave/fs/file_table.c 2007-09-19 10:39:34.0 -0700
@@ -137,6 +137,18 @@ fail:
EXPORT_SYMBOL(get_empty_filp);
+/**
+ * alloc_file - allocate and initialize a 'struct file' with
+ * the given arguments
* function_name - short description (on one line)
* @param1: descr1
* @param2: descr2
etc.
+ *
+ * Use this instead of get_empty_filp() to get a new
+ * 'struct file'. Do so because of the same initialization
+ * pitfalls reasons listed for init_file(). This is a
+ * preferred interface to using init_file().
+ *
+ * If all the callers of init_file() are eliminated, its
+ * code should be moved into this function.
+ */
struct file *alloc_file(struct vfsmount *mnt, struct dentry *dentry,
mode_t mode, const struct file_operations *fop)
{
@@ -152,7 +164,15 @@ struct file *alloc_file(struct vfsmount
}
EXPORT_SYMBOL(alloc_file);
-/*
+/**
+ * init_file - initialize a 'struct file'
+ * @file: the already allocated 'struct file' to initialized
+ * @mnt: the vfsmount on which the file resides
+ *
+ * Use this instead of setting the members directly. Doing so
+ * avoids making mistakes like forgetting the mntget() or
+ * forgetting to take a write on the mnt.
+ *
* Note: This is a crappy interface. It is here to make
* merging with the existing users of get_empty_filp()
* who have complex failure logic easier. All users
OK, that one looks better. :)
---
~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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
