2011-06-06 20:20:25

by Hartley Sweeten

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

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 <[email protected]>

---

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


2011-06-06 21:07:57

by Al Viro

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

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 <[email protected]>

Nacked-by: Al Viro <[email protected]>
Nacked-because: too ugly

Don't play that kind of games with preprocessor.

2011-06-06 21:18:21

by Alexey Dobriyan

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

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 <[email protected]>
>
> Nacked-by: Al Viro <[email protected]>
> 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 :^)

2011-06-06 21:19:26

by Hartley Sweeten

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

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 <[email protected]>
>
> Nacked-by: Al Viro <[email protected]>
> 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

2011-06-06 21:22:14

by Al Viro

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

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 <[email protected]>
> >
> > Nacked-by: Al Viro <[email protected]>
> > 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.

2011-06-06 21:26:54

by Arnd Bergmann

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

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