[PATCH] seq_file.h: introduce DECLARE_SEQ_FOPS_{RO,RW}

[PATCH] seq_file.h: introduce DECLARE_SEQ_FOPS_{RO,RW}

[H Hartley Sweeten]

Many of the procfs and debugfs attribute file_operations in the kernel are
missing the .owner information. Introduce some macro's to fill in the .owner
field as well as the common methods for virtual file file_operations.  This
simplifies creating the attributes and makes sure all the fields are properly
initialized.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>

---

diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index 03c0232..6a1f991 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -153,4 +153,28 @@ extern struct hlist_node *seq_hlist_start_head_rcu(struct hlist_head *head,
 extern struct hlist_node *seq_hlist_next_rcu(void *v,
 						   struct hlist_head *head,
 						   loff_t *ppos);
+
+/*
+ * virtual filesystem attribute files
+ */
+
+#define DECLARE_SEQ_FOPS_RO(name)			\
+static const struct file_operations name##_fops = {	\
+	.owner		= THIS_MODULE,			\
+	.llseek		= seq_lseek,			\
+	.read		= seq_read,			\
+	.open		= name##_open,			\
+	.release	= single_release,		\
+}
+
+#define DECLARE_SEQ_FOPS_RW(name)			\
+static const struct file_operations name##_fops = {	\
+	.owner		= THIS_MODULE,			\
+	.llseek		= seq_lseek,			\
+	.read		= seq_read,			\
+	.write		= name##_write,			\
+	.open		= name##_open,			\
+	.release	= single_release,		\
+}
+
 #endif

Re: [PATCH] seq_file.h: introduce DECLARE_SEQ_FOPS_{RO,RW}

[Al Viro]

On Mon, Jun 06, 2011 at 01:19:45PM -0700, H Hartley Sweeten wrote:
> Many of the procfs and debugfs attribute file_operations in the kernel are
> missing the .owner information. Introduce some macro's to fill in the .owner
> field as well as the common methods for virtual file file_operations.  This
> simplifies creating the attributes and makes sure all the fields are properly
> initialized.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>

Nacked-by: Al Viro <viro@zeniv.linux.org.uk>
Nacked-because: too ugly

Don't play that kind of games with preprocessor.

Re: [PATCH] seq_file.h: introduce DECLARE_SEQ_FOPS_{RO,RW}

[Alexey Dobriyan]

On Mon, Jun 06, 2011 at 10:07:55PM +0100, Al Viro wrote:
> On Mon, Jun 06, 2011 at 01:19:45PM -0700, H Hartley Sweeten wrote:
> > Many of the procfs and debugfs attribute file_operations in the kernel are
> > missing the .owner information. Introduce some macro's to fill in the .owner
> > field as well as the common methods for virtual file file_operations.  This
> > simplifies creating the attributes and makes sure all the fields are properly
> > initialized.
> > 
> > Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> 
> Nacked-by: Al Viro <viro@zeniv.linux.org.uk>
> Nacked-because: too ugly
> 
> Don't play that kind of games with preprocessor.

3) .owner is ignored for procfs
4) ## breaks grep (and tags)
5) macro name doesn't hint for important "single" piece.
6) DECLARE and DEFINE semi-idioms are used another way :^)

RE: [PATCH] seq_file.h: introduce DECLARE_SEQ_FOPS_{RO,RW}

[H Hartley Sweeten]

On Monday, June 06, 2011 2:08 PM, Al Viro wrote:
> On Mon, Jun 06, 2011 at 01:19:45PM -0700, H Hartley Sweeten wrote:
>> Many of the procfs and debugfs attribute file_operations in the kernel are
>> missing the .owner information. Introduce some macro's to fill in the .owner
>> field as well as the common methods for virtual file file_operations.  This
>> simplifies creating the attributes and makes sure all the fields are properly
>> initialized.
>> 
>> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>
> Nacked-by: Al Viro <viro@zeniv.linux.org.uk>
> Nacked-because: too ugly
>
> Don't play that kind of games with preprocessor.

Just an FYI, the exact same thing is being done with DEFINE_SIMPLE_ATTRIBUTE
in fs.h:

#define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt)		\
static int __fops ## _open(struct inode *inode, struct file *file)	\
{									\
	__simple_attr_check_format(__fmt, 0ull);			\
	return simple_attr_open(inode, file, __get, __set, __fmt);	\
}									\
static const struct file_operations __fops = {				\
	.owner	 = THIS_MODULE,						\
	.open	 = __fops ## _open,					\
	.release = simple_attr_release,					\
	.read	 = simple_attr_read,					\
	.write	 = simple_attr_write,					\
	.llseek	 = generic_file_llseek,					\
};

Regards,
Hartley

Re: [PATCH] seq_file.h: introduce DECLARE_SEQ_FOPS_{RO,RW}

[Al Viro]

On Mon, Jun 06, 2011 at 04:19:15PM -0500, H Hartley Sweeten wrote:
> On Monday, June 06, 2011 2:08 PM, Al Viro wrote:
> > On Mon, Jun 06, 2011 at 01:19:45PM -0700, H Hartley Sweeten wrote:
> >> Many of the procfs and debugfs attribute file_operations in the kernel are
> >> missing the .owner information. Introduce some macro's to fill in the .owner
> >> field as well as the common methods for virtual file file_operations.  This
> >> simplifies creating the attributes and makes sure all the fields are properly
> >> initialized.
> >> 
> >> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> >
> > Nacked-by: Al Viro <viro@zeniv.linux.org.uk>
> > Nacked-because: too ugly
> >
> > Don't play that kind of games with preprocessor.
> 
> Just an FYI, the exact same thing is being done with DEFINE_SIMPLE_ATTRIBUTE
> in fs.h:

Yes, and DEFINE_SIMPLE_ATTRIBUTE is sick and ugly.

Re: [PATCH] seq_file.h: introduce DECLARE_SEQ_FOPS_{RO,RW}

[Arnd Bergmann]

On Monday 06 June 2011 23:19:15 H Hartley Sweeten wrote:
> Just an FYI, the exact same thing is being done with DEFINE_SIMPLE_ATTRIBUTE
> in fs.h:
> 
> #define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt)            \
> static int __fops ## _open(struct inode *inode, struct file *file)      \
> {                                                                       \
>         __simple_attr_check_format(__fmt, 0ull);                        \
>         return simple_attr_open(inode, file, __get, __set, __fmt);      \
> }                                                                       \
> static const struct file_operations __fops = {                          \
>         .owner   = THIS_MODULE,                                         \
>         .open    = __fops ## _open,                                     \
>         .release = simple_attr_release,                                 \
>         .read    = simple_attr_read,                                    \
>         .write   = simple_attr_write,                                   \
>         .llseek  = generic_file_llseek,                                 \
> };

The main difference here is that the arguments to the macro are used directly
in the file operations, which makes it possible to grep for them.

	Arnd