Skip to contentSkip to conversation

seq_file, Bluetooth: introduce DEFINE_SHOW_ATTRIBUTE()

PATCH In review Andy Shevchenko 3 participants 6 messages Nov 23, 2017 – Dec 13, 2017 1 message not in the archive

Subsystems (4)
Networking Network drivers Bluetooth drivers Bluetooth
lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

v2 cover seq_file, Bluetooth: introduce DEFINE_SHOW_ATTRIBUTE()

Introduce a helper macro for attributes that have only ->show() defined.

P.S. I have few more clean ups locally, thus it would be nice to have an immutable branch for the rest patches which would like to utilize the new macro.

In v2:
- introduce for all users followed by an example clean up of hci_debugfs.c

Andy Shevchenko (2):
seq_file: Introduce DEFINE_SHOW_ATTRIBUTE() helper macro
Bluetooth: Re-use DEFINE_SHOW_ATTRIBUTE() macro

 include/linux/seq_file.h    |  13 +++
 net/bluetooth/hci_debugfs.c | 193 ++++----------------------------------------
 2 files changed, 29 insertions(+), 177 deletions(-)
Signature · 1 line
2.15.0
author
#1 lore raw

PATCH v2 1/2 seq_file: Introduce DEFINE_SHOW_ATTRIBUTE() helper macro

The DEFINE_SHOW_ATTRIBUTE() helper macro would be useful for current users, which are many of them, and for new comers to decrease code duplication.

Signed-off-by Andy Shevchenko <[email protected]>
---
 include/linux/seq_file.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)
include/linux/seq_file.h +13 -0
index c32929802c25..73adc12ab773 100644
@@ -144,6 +144,19 @@ void *__seq_open_private(struct file *, const struct seq_operations *, int);
 int seq_open_private(struct file *, const struct seq_operations *, int);
 int seq_release_private(struct inode *, struct file *);
 
+#define DEFINE_SHOW_ATTRIBUTE(__name)					      \
+static int __name ## _open(struct inode *inode, struct file *file)	      \
+{									      \
+	return single_open(file, __name ## _show, inode->i_private);	      \
+}									      \
+									      \
+static const struct file_operations __name ## _fops = {			      \
+	.open		= __name ## _open,				      \
+	.read		= seq_read,					      \
+	.llseek		= seq_lseek,					      \
+	.release	= single_release,				      \
+}									      \
+
 static inline struct user_namespace *seq_user_ns(struct seq_file *seq)
 {
 #ifdef CONFIG_USER_NS
Signature · 1 line
2.15.0
maintainer
#2 lore raw

Re: [PATCH v2 1/2] seq_file: Introduce DEFINE_SHOW_ATTRIBUTE() helper macro

Hi Andy,

Quoted 20 lines

The DEFINE_SHOW_ATTRIBUTE() helper macro would be useful for current users, which are many of them, and for new comers to decrease code duplication.

Signed-off-by Andy Shevchenko <[email protected]>

---
include/linux/seq_file.h | 13 +++++++++++++

1 file changed, 13 insertions(+)
include/linux/seq_file.h +3 -0
index c32929802c25..73adc12ab773 100644
@@ -144,6 +144,19 @@ void *__seq_open_private(struct file *, const struct seq_operations *, int);
 int seq_open_private(struct file *, const struct seq_operations *, int);
 int seq_release_private(struct inode *, struct file *);
 
+#define DEFINE_SHOW_ATTRIBUTE(__name)					      \
+static int __name ## _open(struct inode *inode, struct file *file)	      \
+{									      \

include/linux/seq_file.h void *__seq_open_private(struct file *, const struct seq_operations *, int); · line 159

Quoted 10 lines
include/linux/seq_file.h +10 -0
+	return single_open(file, __name ## _show, inode->i_private);	      \
+}									      \
+									      \
+static const struct file_operations __name ## _fops = {			      \
+	.open		= __name ## _open,				      \
+	.read		= seq_read,					      \
+	.llseek		= seq_lseek,					      \
+	.release	= single_release,				      \
+}									      \
+

can we define this in hci_debugfs.c first and get that patch merged into bluetooth-next. And only as a follow up patch try to generalize this in seq_file.h. I really don’t like to carry a patch for seq_file.h in the bluetooth-next tree.

Regards

Marcel

A message that is not in the archive (sent off-list or not yet received): F129B4E0-91B1-4B7B-AF5D-A5EA29486A6E-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org

#4 lore raw

Re: [PATCH v2 1/2] seq_file: Introduce DEFINE_SHOW_ATTRIBUTE() helper macro

Quoted 11 lines · Marcel Holtmann

Hi Andy,

Quoted 3 lines

The DEFINE_SHOW_ATTRIBUTE() helper macro would be useful for current users, which are many of them, and for new comers to decrease code duplication.

can we define this in hci_debugfs.c first and get that patch merged into bluetooth-next. And only as a follow up patch try to generalize this in seq_file.h. I really don’t like to carry a patch for seq_file.h in the bluetooth-next tree.

This was in v1 [1].

Feel free to apply it instead. Frankly I don't understand what's wrong with carrying patch against seq_file.h? Is it "person non grata"?

[1]: https://marc.info/?l=linux-bluetooth&m=151138535801354&w=2

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
author
#5 lore raw

Re: [PATCH v2 1/2] seq_file: Introduce DEFINE_SHOW_ATTRIBUTE() helper macro

Quoted 13 lines · Andy Shevchenko
Quoted 11 lines · Marcel Holtmann

Hi Andy,

Quoted 4 lines

The DEFINE_SHOW_ATTRIBUTE() helper macro would be useful for current
users, which are many of them, and for new comers to decrease code duplication.

can we define this in hci_debugfs.c first and get that patch merged into bluetooth-next. And only as a follow up patch try to generalize this in seq_file.h. I really don’t like to carry a patch for seq_file.h in the bluetooth-next tree.

Quoted 6 lines

This was in v1 [1].

Feel free to apply it instead. Frankly I don't understand what's wrong with carrying patch against seq_file.h? Is it "person non grata"?

[1]: https://marc.info/?l=linux-bluetooth&m=151138535801354&w=2

Marcel, can you apply v1? Or should I resend it as v3?

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
author
#6 lore raw

PATCH v2 2/2 Bluetooth: Re-use DEFINE_SHOW_ATTRIBUTE() macro

In reply to Andy Shevchenko

...instead of open coding file operations followed by custom ->open() callbacks per each attribute.

Signed-off-by Andy Shevchenko <[email protected]>
---
 net/bluetooth/hci_debugfs.c | 193 ++++----------------------------------------
 1 file changed, 16 insertions(+), 177 deletions(-)
net/bluetooth/hci_debugfs.c +16 -177
index 63df63ebfb24..2fffc5a05243 100644
@@ -88,17 +88,7 @@ static int __name ## _show(struct seq_file *f, void *ptr)		      \
 	return 0;							      \
 }									      \
 									      \
-static int __name ## _open(struct inode *inode, struct file *file)	      \
-{									      \
-	return single_open(file, __name ## _show, inode->i_private);	      \
-}									      \
-									      \
-static const struct file_operations __name ## _fops = {			      \
-	.open		= __name ## _open,				      \
-	.read		= seq_read,					      \
-	.llseek		= seq_lseek,					      \
-	.release	= single_release,				      \
-}									      \
+DEFINE_SHOW_ATTRIBUTE(__name)
 
 static int features_show(struct seq_file *f, void *ptr)
 {

@@ -126,17 +116,7 @@ static int features_show(struct seq_file *f, void *ptr)
 	return 0;
 }
 
-static int features_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, features_show, inode->i_private);
-}
-
-static const struct file_operations features_fops = {
-	.open		= features_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(features);
 
 static int device_id_show(struct seq_file *f, void *ptr)
 {

@@ -150,17 +130,7 @@ static int device_id_show(struct seq_file *f, void *ptr)
 	return 0;
 }
 
-static int device_id_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, device_id_show, inode->i_private);
-}
-
-static const struct file_operations device_id_fops = {
-	.open		= device_id_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(device_id);
 
 static int device_list_show(struct seq_file *f, void *ptr)
 {

@@ -180,17 +150,7 @@ static int device_list_show(struct seq_file *f, void *ptr)
 	return 0;
 }
 
-static int device_list_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, device_list_show, inode->i_private);
-}
-
-static const struct file_operations device_list_fops = {
-	.open		= device_list_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(device_list);
 
 static int blacklist_show(struct seq_file *f, void *p)
 {

@@ -205,17 +165,7 @@ static int blacklist_show(struct seq_file *f, void *p)
 	return 0;
 }
 
-static int blacklist_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, blacklist_show, inode->i_private);
-}
-
-static const struct file_operations blacklist_fops = {
-	.open		= blacklist_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(blacklist);
 
 static int uuids_show(struct seq_file *f, void *p)
 {

@@ -240,17 +190,7 @@ static int uuids_show(struct seq_file *f, void *p)
        return 0;
 }
 
-static int uuids_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, uuids_show, inode->i_private);
-}
-
-static const struct file_operations uuids_fops = {
-	.open		= uuids_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(uuids);
 
 static int remote_oob_show(struct seq_file *f, void *ptr)
 {

@@ -269,17 +209,7 @@ static int remote_oob_show(struct seq_file *f, void *ptr)
 	return 0;
 }
 
-static int remote_oob_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, remote_oob_show, inode->i_private);
-}
-
-static const struct file_operations remote_oob_fops = {
-	.open		= remote_oob_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(remote_oob);
 
 static int conn_info_min_age_set(void *data, u64 val)
 {

@@ -443,17 +373,7 @@ static int inquiry_cache_show(struct seq_file *f, void *p)
 	return 0;
 }
 
-static int inquiry_cache_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, inquiry_cache_show, inode->i_private);
-}
-
-static const struct file_operations inquiry_cache_fops = {
-	.open		= inquiry_cache_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(inquiry_cache);
 
 static int link_keys_show(struct seq_file *f, void *ptr)
 {

@@ -469,17 +389,7 @@ static int link_keys_show(struct seq_file *f, void *ptr)
 	return 0;
 }
 
-static int link_keys_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, link_keys_show, inode->i_private);
-}
-
-static const struct file_operations link_keys_fops = {
-	.open		= link_keys_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(link_keys);
 
 static int dev_class_show(struct seq_file *f, void *ptr)
 {

@@ -493,17 +403,7 @@ static int dev_class_show(struct seq_file *f, void *ptr)
 	return 0;
 }
 
-static int dev_class_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, dev_class_show, inode->i_private);
-}
-
-static const struct file_operations dev_class_fops = {
-	.open		= dev_class_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(dev_class);
 
 static int voice_setting_get(void *data, u64 *val)
 {

@@ -692,17 +592,7 @@ static int identity_show(struct seq_file *f, void *p)
 	return 0;
 }
 
-static int identity_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, identity_show, inode->i_private);
-}
-
-static const struct file_operations identity_fops = {
-	.open		= identity_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(identity);
 
 static int rpa_timeout_set(void *data, u64 val)
 {

@@ -746,17 +636,7 @@ static int random_address_show(struct seq_file *f, void *p)
 	return 0;
 }
 
-static int random_address_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, random_address_show, inode->i_private);
-}
-
-static const struct file_operations random_address_fops = {
-	.open		= random_address_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(random_address);
 
 static int static_address_show(struct seq_file *f, void *p)
 {

@@ -769,17 +649,7 @@ static int static_address_show(struct seq_file *f, void *p)
 	return 0;
 }
 
-static int static_address_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, static_address_show, inode->i_private);
-}
-
-static const struct file_operations static_address_fops = {
-	.open		= static_address_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(static_address);
 
 static ssize_t force_static_address_read(struct file *file,
 					 char __user *user_buf,

@@ -841,17 +711,7 @@ static int white_list_show(struct seq_file *f, void *ptr)
 	return 0;
 }
 
-static int white_list_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, white_list_show, inode->i_private);
-}
-
-static const struct file_operations white_list_fops = {
-	.open		= white_list_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(white_list);
 
 static int identity_resolving_keys_show(struct seq_file *f, void *ptr)
 {

@@ -869,18 +729,7 @@ static int identity_resolving_keys_show(struct seq_file *f, void *ptr)
 	return 0;
 }
 
-static int identity_resolving_keys_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, identity_resolving_keys_show,
-			   inode->i_private);
-}
-
-static const struct file_operations identity_resolving_keys_fops = {
-	.open		= identity_resolving_keys_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(identity_resolving_keys);
 
 static int long_term_keys_show(struct seq_file *f, void *ptr)
 {

@@ -898,17 +747,7 @@ static int long_term_keys_show(struct seq_file *f, void *ptr)
 	return 0;
 }
 
-static int long_term_keys_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, long_term_keys_show, inode->i_private);
-}
-
-static const struct file_operations long_term_keys_fops = {
-	.open		= long_term_keys_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(long_term_keys);
 
 static int conn_min_interval_set(void *data, u64 val)
 {
Signature · 1 line
2.15.0