Skip to contentSkip to conversation

seq_file.h: introduce DECLARE_SEQ_FOPS_{RO,RW}

PATCH NAK H Hartley Sweeten 4 participants 6 messages Jun 6, 2011

lore
Share & tools
lore

H Hartley Sweeten

author
#0 lore raw

PATCH 1/1 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]>

---

include/linux/seq_file.h +24 -0
index 03c0232..6a1f991 100644
@@ -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
Nacked-by on v1 1/1 NAK
#1 lore raw
Quoted 7 lines · 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 <[email protected]>
Nacked-by Al Viro <[email protected]>

Nacked-because: too ugly

Don't play that kind of games with preprocessor.

#2 lore raw
Quoted 7 lines · Al Viro
Quoted 6 lines · 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.

Quoted 6 lines
Quoted 1 line
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 :^)

author
#3 lore raw
In reply to Al Viro
Quoted 7 lines · Al Viro
Quoted 6 lines · 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.

Quoted 6 lines
Quoted 1 line
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

#4 lore raw
Quoted 10 lines · H Hartley Sweeten
Quoted 9 lines · Al Viro
Quoted 7 lines · 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 <[email protected]>
Quoted 7 lines
Quoted 4 lines
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.

#5 lore raw
In reply to H Hartley Sweeten
Quoted 16 lines · 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,                                 \
Quoted 1 line

};

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