* [PATCH 0/7] seq_printf cleanups [not found] <20140929124246.3e39dac8@gandalf.local.home> @ 2014-09-29 23:08 ` Joe Perches 2014-09-29 23:08 ` [PATCH 7/7] docg3: Fix miuse of seq_printf return value Joe Perches 2014-10-28 15:32 ` [PATCH 0/7] seq_printf cleanups Steven Rostedt 0 siblings, 2 replies; 14+ messages in thread From: Joe Perches @ 2014-09-29 23:08 UTC (permalink / raw) To: Steven Rostedt Cc: linux-doc, netdev, linux-kernel, Petr Mladek, cluster-devel, coreteam, linux-mtd, Al Viro, linux-fsdevel, netfilter-devel, Andrew Morton, Linus Torvalds seq_printf should return void. Add a public bool seq_is_full function that can be used to shortcut unnecesary seq_printf/seq_puts calls when the seq buffer is full. Start removing the misuses of the seq_printf/seq_puts return value. Patchset brought forward from an unreplied to set of changes from back in December 2013. https://lkml.org/lkml/2013/12/11/713 Renamed seq_is_buf_full to seq_is_full. Joe Perches (7): seq_file: Rename static bool seq_overflow to public bool seq_is_full netfilter: Convert print_tuple functions to return void dlm: Use seq_is_full - remove seq_printf returns dlm: Use seq_puts, remove unnecessary trailing spaces fs: Convert show_fdinfo functions to void debugfs: Fix misuse of seq_printf return value docg3: Fix mixuse of seq_printf return value Documentation/filesystems/seq_file.txt | 28 +-- Documentation/filesystems/vfs.txt | 2 +- drivers/mtd/devices/docg3.c | 112 ++++++------ drivers/net/tun.c | 4 +- fs/debugfs/file.c | 14 +- fs/dlm/debug_fs.c | 260 +++++++++++++-------------- fs/eventfd.c | 15 +- fs/eventpoll.c | 19 +- fs/notify/fdinfo.c | 76 ++++---- fs/notify/fdinfo.h | 4 +- fs/proc/fd.c | 2 +- fs/seq_file.c | 28 +-- fs/signalfd.c | 10 +- fs/timerfd.c | 27 +-- include/linux/fs.h | 2 +- include/linux/seq_file.h | 8 + include/net/netfilter/nf_conntrack_core.h | 2 +- include/net/netfilter/nf_conntrack_l3proto.h | 4 +- include/net/netfilter/nf_conntrack_l4proto.h | 4 +- net/netfilter/nf_conntrack_l3proto_generic.c | 5 +- net/netfilter/nf_conntrack_proto_dccp.c | 10 +- net/netfilter/nf_conntrack_proto_generic.c | 5 +- net/netfilter/nf_conntrack_proto_gre.c | 10 +- net/netfilter/nf_conntrack_proto_sctp.c | 10 +- net/netfilter/nf_conntrack_proto_tcp.c | 10 +- net/netfilter/nf_conntrack_proto_udp.c | 10 +- net/netfilter/nf_conntrack_proto_udplite.c | 10 +- net/netfilter/nf_conntrack_standalone.c | 15 +- 28 files changed, 333 insertions(+), 373 deletions(-) -- 1.8.1.2.459.gbcd45b4.dirty ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 7/7] docg3: Fix miuse of seq_printf return value 2014-09-29 23:08 ` [PATCH 0/7] seq_printf cleanups Joe Perches @ 2014-09-29 23:08 ` Joe Perches 2014-10-22 8:29 ` Brian Norris 2014-10-28 15:32 ` [PATCH 0/7] seq_printf cleanups Steven Rostedt 1 sibling, 1 reply; 14+ messages in thread From: Joe Perches @ 2014-09-29 23:08 UTC (permalink / raw) To: Steven Rostedt Cc: Linus Torvalds, linux-kernel, Petr Mladek, linux-mtd, Al Viro, Andrew Morton, Brian Norris, David Woodhouse seq_printf doesn't return a useful value, so remove these misuses. Signed-off-by: Joe Perches <joe@perches.com> --- drivers/mtd/devices/docg3.c | 112 ++++++++++++++++++++------------------------ 1 file changed, 52 insertions(+), 60 deletions(-) diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c index 21cc4b6..68ff83c 100644 --- a/drivers/mtd/devices/docg3.c +++ b/drivers/mtd/devices/docg3.c @@ -1655,22 +1655,21 @@ static int dbg_flashctrl_show(struct seq_file *s, void *p) { struct docg3 *docg3 = (struct docg3 *)s->private; - int pos = 0; u8 fctrl; mutex_lock(&docg3->cascade->lock); fctrl = doc_register_readb(docg3, DOC_FLASHCONTROL); mutex_unlock(&docg3->cascade->lock); - pos += seq_printf(s, - "FlashControl : 0x%02x (%s,CE# %s,%s,%s,flash %s)\n", - fctrl, - fctrl & DOC_CTRL_VIOLATION ? "protocol violation" : "-", - fctrl & DOC_CTRL_CE ? "active" : "inactive", - fctrl & DOC_CTRL_PROTECTION_ERROR ? "protection error" : "-", - fctrl & DOC_CTRL_SEQUENCE_ERROR ? "sequence error" : "-", - fctrl & DOC_CTRL_FLASHREADY ? "ready" : "not ready"); - return pos; + seq_printf(s, "FlashControl : 0x%02x (%s,CE# %s,%s,%s,flash %s)\n", + fctrl, + fctrl & DOC_CTRL_VIOLATION ? "protocol violation" : "-", + fctrl & DOC_CTRL_CE ? "active" : "inactive", + fctrl & DOC_CTRL_PROTECTION_ERROR ? "protection error" : "-", + fctrl & DOC_CTRL_SEQUENCE_ERROR ? "sequence error" : "-", + fctrl & DOC_CTRL_FLASHREADY ? "ready" : "not ready"); + + return 0; } DEBUGFS_RO_ATTR(flashcontrol, dbg_flashctrl_show); @@ -1678,58 +1677,56 @@ static int dbg_asicmode_show(struct seq_file *s, void *p) { struct docg3 *docg3 = (struct docg3 *)s->private; - int pos = 0, pctrl, mode; + int pctrl, mode; mutex_lock(&docg3->cascade->lock); pctrl = doc_register_readb(docg3, DOC_ASICMODE); mode = pctrl & 0x03; mutex_unlock(&docg3->cascade->lock); - pos += seq_printf(s, - "%04x : RAM_WE=%d,RSTIN_RESET=%d,BDETCT_RESET=%d,WRITE_ENABLE=%d,POWERDOWN=%d,MODE=%d%d (", - pctrl, - pctrl & DOC_ASICMODE_RAM_WE ? 1 : 0, - pctrl & DOC_ASICMODE_RSTIN_RESET ? 1 : 0, - pctrl & DOC_ASICMODE_BDETCT_RESET ? 1 : 0, - pctrl & DOC_ASICMODE_MDWREN ? 1 : 0, - pctrl & DOC_ASICMODE_POWERDOWN ? 1 : 0, - mode >> 1, mode & 0x1); + seq_printf(s, + "%04x : RAM_WE=%d,RSTIN_RESET=%d,BDETCT_RESET=%d,WRITE_ENABLE=%d,POWERDOWN=%d,MODE=%d%d (", + pctrl, + pctrl & DOC_ASICMODE_RAM_WE ? 1 : 0, + pctrl & DOC_ASICMODE_RSTIN_RESET ? 1 : 0, + pctrl & DOC_ASICMODE_BDETCT_RESET ? 1 : 0, + pctrl & DOC_ASICMODE_MDWREN ? 1 : 0, + pctrl & DOC_ASICMODE_POWERDOWN ? 1 : 0, + mode >> 1, mode & 0x1); switch (mode) { case DOC_ASICMODE_RESET: - pos += seq_puts(s, "reset"); + seq_puts(s, "reset"); break; case DOC_ASICMODE_NORMAL: - pos += seq_puts(s, "normal"); + seq_puts(s, "normal"); break; case DOC_ASICMODE_POWERDOWN: - pos += seq_puts(s, "powerdown"); + seq_puts(s, "powerdown"); break; } - pos += seq_puts(s, ")\n"); - return pos; + seq_puts(s, ")\n"); + return 0; } DEBUGFS_RO_ATTR(asic_mode, dbg_asicmode_show); static int dbg_device_id_show(struct seq_file *s, void *p) { struct docg3 *docg3 = (struct docg3 *)s->private; - int pos = 0; int id; mutex_lock(&docg3->cascade->lock); id = doc_register_readb(docg3, DOC_DEVICESELECT); mutex_unlock(&docg3->cascade->lock); - pos += seq_printf(s, "DeviceId = %d\n", id); - return pos; + seq_printf(s, "DeviceId = %d\n", id); + return 0; } DEBUGFS_RO_ATTR(device_id, dbg_device_id_show); static int dbg_protection_show(struct seq_file *s, void *p) { struct docg3 *docg3 = (struct docg3 *)s->private; - int pos = 0; int protect, dps0, dps0_low, dps0_high, dps1, dps1_low, dps1_high; mutex_lock(&docg3->cascade->lock); @@ -1742,45 +1739,40 @@ static int dbg_protection_show(struct seq_file *s, void *p) dps1_high = doc_register_readw(docg3, DOC_DPS1_ADDRHIGH); mutex_unlock(&docg3->cascade->lock); - pos += seq_printf(s, "Protection = 0x%02x (", - protect); + seq_printf(s, "Protection = 0x%02x (", protect); if (protect & DOC_PROTECT_FOUNDRY_OTP_LOCK) - pos += seq_puts(s, "FOUNDRY_OTP_LOCK,"); + seq_puts(s, "FOUNDRY_OTP_LOCK,"); if (protect & DOC_PROTECT_CUSTOMER_OTP_LOCK) - pos += seq_puts(s, "CUSTOMER_OTP_LOCK,"); + seq_puts(s, "CUSTOMER_OTP_LOCK,"); if (protect & DOC_PROTECT_LOCK_INPUT) - pos += seq_puts(s, "LOCK_INPUT,"); + seq_puts(s, "LOCK_INPUT,"); if (protect & DOC_PROTECT_STICKY_LOCK) - pos += seq_puts(s, "STICKY_LOCK,"); + seq_puts(s, "STICKY_LOCK,"); if (protect & DOC_PROTECT_PROTECTION_ENABLED) - pos += seq_puts(s, "PROTECTION ON,"); + seq_puts(s, "PROTECTION ON,"); if (protect & DOC_PROTECT_IPL_DOWNLOAD_LOCK) - pos += seq_puts(s, "IPL_DOWNLOAD_LOCK,"); + seq_puts(s, "IPL_DOWNLOAD_LOCK,"); if (protect & DOC_PROTECT_PROTECTION_ERROR) - pos += seq_puts(s, "PROTECT_ERR,"); + seq_puts(s, "PROTECT_ERR,"); else - pos += seq_puts(s, "NO_PROTECT_ERR"); - pos += seq_puts(s, ")\n"); - - pos += seq_printf(s, "DPS0 = 0x%02x : " - "Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, " - "WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n", - dps0, dps0_low, dps0_high, - !!(dps0 & DOC_DPS_OTP_PROTECTED), - !!(dps0 & DOC_DPS_READ_PROTECTED), - !!(dps0 & DOC_DPS_WRITE_PROTECTED), - !!(dps0 & DOC_DPS_HW_LOCK_ENABLED), - !!(dps0 & DOC_DPS_KEY_OK)); - pos += seq_printf(s, "DPS1 = 0x%02x : " - "Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, " - "WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n", - dps1, dps1_low, dps1_high, - !!(dps1 & DOC_DPS_OTP_PROTECTED), - !!(dps1 & DOC_DPS_READ_PROTECTED), - !!(dps1 & DOC_DPS_WRITE_PROTECTED), - !!(dps1 & DOC_DPS_HW_LOCK_ENABLED), - !!(dps1 & DOC_DPS_KEY_OK)); - return pos; + seq_puts(s, "NO_PROTECT_ERR"); + seq_puts(s, ")\n"); + + seq_printf(s, "DPS0 = 0x%02x : Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n", + dps0, dps0_low, dps0_high, + !!(dps0 & DOC_DPS_OTP_PROTECTED), + !!(dps0 & DOC_DPS_READ_PROTECTED), + !!(dps0 & DOC_DPS_WRITE_PROTECTED), + !!(dps0 & DOC_DPS_HW_LOCK_ENABLED), + !!(dps0 & DOC_DPS_KEY_OK)); + seq_printf(s, "DPS1 = 0x%02x : Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n", + dps1, dps1_low, dps1_high, + !!(dps1 & DOC_DPS_OTP_PROTECTED), + !!(dps1 & DOC_DPS_READ_PROTECTED), + !!(dps1 & DOC_DPS_WRITE_PROTECTED), + !!(dps1 & DOC_DPS_HW_LOCK_ENABLED), + !!(dps1 & DOC_DPS_KEY_OK)); + return 0; } DEBUGFS_RO_ATTR(protection, dbg_protection_show); -- 1.8.1.2.459.gbcd45b4.dirty ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 7/7] docg3: Fix miuse of seq_printf return value 2014-09-29 23:08 ` [PATCH 7/7] docg3: Fix miuse of seq_printf return value Joe Perches @ 2014-10-22 8:29 ` Brian Norris 2014-10-28 15:13 ` Steven Rostedt 0 siblings, 1 reply; 14+ messages in thread From: Brian Norris @ 2014-10-22 8:29 UTC (permalink / raw) To: Joe Perches Cc: Linus Torvalds, linux-kernel, Petr Mladek, Steven Rostedt, Al Viro, linux-mtd, Andrew Morton, David Woodhouse On Mon, Sep 29, 2014 at 04:08:27PM -0700, Joe Perches wrote: > seq_printf doesn't return a useful value, so remove > these misuses. Good catch. So it looks like this driver always had some form of wrongness (returning a character count) in its debugfs callbacks, but nobody noticed. Applied to l2-mtd.git. Thanks! Brian > Signed-off-by: Joe Perches <joe@perches.com> > --- > drivers/mtd/devices/docg3.c | 112 ++++++++++++++++++++------------------------ > 1 file changed, 52 insertions(+), 60 deletions(-) > > diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c > index 21cc4b6..68ff83c 100644 > --- a/drivers/mtd/devices/docg3.c > +++ b/drivers/mtd/devices/docg3.c > @@ -1655,22 +1655,21 @@ static int dbg_flashctrl_show(struct seq_file *s, void *p) > { > struct docg3 *docg3 = (struct docg3 *)s->private; > > - int pos = 0; > u8 fctrl; > > mutex_lock(&docg3->cascade->lock); > fctrl = doc_register_readb(docg3, DOC_FLASHCONTROL); > mutex_unlock(&docg3->cascade->lock); > > - pos += seq_printf(s, > - "FlashControl : 0x%02x (%s,CE# %s,%s,%s,flash %s)\n", > - fctrl, > - fctrl & DOC_CTRL_VIOLATION ? "protocol violation" : "-", > - fctrl & DOC_CTRL_CE ? "active" : "inactive", > - fctrl & DOC_CTRL_PROTECTION_ERROR ? "protection error" : "-", > - fctrl & DOC_CTRL_SEQUENCE_ERROR ? "sequence error" : "-", > - fctrl & DOC_CTRL_FLASHREADY ? "ready" : "not ready"); > - return pos; > + seq_printf(s, "FlashControl : 0x%02x (%s,CE# %s,%s,%s,flash %s)\n", > + fctrl, > + fctrl & DOC_CTRL_VIOLATION ? "protocol violation" : "-", > + fctrl & DOC_CTRL_CE ? "active" : "inactive", > + fctrl & DOC_CTRL_PROTECTION_ERROR ? "protection error" : "-", > + fctrl & DOC_CTRL_SEQUENCE_ERROR ? "sequence error" : "-", > + fctrl & DOC_CTRL_FLASHREADY ? "ready" : "not ready"); > + > + return 0; > } > DEBUGFS_RO_ATTR(flashcontrol, dbg_flashctrl_show); > > @@ -1678,58 +1677,56 @@ static int dbg_asicmode_show(struct seq_file *s, void *p) > { > struct docg3 *docg3 = (struct docg3 *)s->private; > > - int pos = 0, pctrl, mode; > + int pctrl, mode; > > mutex_lock(&docg3->cascade->lock); > pctrl = doc_register_readb(docg3, DOC_ASICMODE); > mode = pctrl & 0x03; > mutex_unlock(&docg3->cascade->lock); > > - pos += seq_printf(s, > - "%04x : RAM_WE=%d,RSTIN_RESET=%d,BDETCT_RESET=%d,WRITE_ENABLE=%d,POWERDOWN=%d,MODE=%d%d (", > - pctrl, > - pctrl & DOC_ASICMODE_RAM_WE ? 1 : 0, > - pctrl & DOC_ASICMODE_RSTIN_RESET ? 1 : 0, > - pctrl & DOC_ASICMODE_BDETCT_RESET ? 1 : 0, > - pctrl & DOC_ASICMODE_MDWREN ? 1 : 0, > - pctrl & DOC_ASICMODE_POWERDOWN ? 1 : 0, > - mode >> 1, mode & 0x1); > + seq_printf(s, > + "%04x : RAM_WE=%d,RSTIN_RESET=%d,BDETCT_RESET=%d,WRITE_ENABLE=%d,POWERDOWN=%d,MODE=%d%d (", > + pctrl, > + pctrl & DOC_ASICMODE_RAM_WE ? 1 : 0, > + pctrl & DOC_ASICMODE_RSTIN_RESET ? 1 : 0, > + pctrl & DOC_ASICMODE_BDETCT_RESET ? 1 : 0, > + pctrl & DOC_ASICMODE_MDWREN ? 1 : 0, > + pctrl & DOC_ASICMODE_POWERDOWN ? 1 : 0, > + mode >> 1, mode & 0x1); > > switch (mode) { > case DOC_ASICMODE_RESET: > - pos += seq_puts(s, "reset"); > + seq_puts(s, "reset"); > break; > case DOC_ASICMODE_NORMAL: > - pos += seq_puts(s, "normal"); > + seq_puts(s, "normal"); > break; > case DOC_ASICMODE_POWERDOWN: > - pos += seq_puts(s, "powerdown"); > + seq_puts(s, "powerdown"); > break; > } > - pos += seq_puts(s, ")\n"); > - return pos; > + seq_puts(s, ")\n"); > + return 0; > } > DEBUGFS_RO_ATTR(asic_mode, dbg_asicmode_show); > > static int dbg_device_id_show(struct seq_file *s, void *p) > { > struct docg3 *docg3 = (struct docg3 *)s->private; > - int pos = 0; > int id; > > mutex_lock(&docg3->cascade->lock); > id = doc_register_readb(docg3, DOC_DEVICESELECT); > mutex_unlock(&docg3->cascade->lock); > > - pos += seq_printf(s, "DeviceId = %d\n", id); > - return pos; > + seq_printf(s, "DeviceId = %d\n", id); > + return 0; > } > DEBUGFS_RO_ATTR(device_id, dbg_device_id_show); > > static int dbg_protection_show(struct seq_file *s, void *p) > { > struct docg3 *docg3 = (struct docg3 *)s->private; > - int pos = 0; > int protect, dps0, dps0_low, dps0_high, dps1, dps1_low, dps1_high; > > mutex_lock(&docg3->cascade->lock); > @@ -1742,45 +1739,40 @@ static int dbg_protection_show(struct seq_file *s, void *p) > dps1_high = doc_register_readw(docg3, DOC_DPS1_ADDRHIGH); > mutex_unlock(&docg3->cascade->lock); > > - pos += seq_printf(s, "Protection = 0x%02x (", > - protect); > + seq_printf(s, "Protection = 0x%02x (", protect); > if (protect & DOC_PROTECT_FOUNDRY_OTP_LOCK) > - pos += seq_puts(s, "FOUNDRY_OTP_LOCK,"); > + seq_puts(s, "FOUNDRY_OTP_LOCK,"); > if (protect & DOC_PROTECT_CUSTOMER_OTP_LOCK) > - pos += seq_puts(s, "CUSTOMER_OTP_LOCK,"); > + seq_puts(s, "CUSTOMER_OTP_LOCK,"); > if (protect & DOC_PROTECT_LOCK_INPUT) > - pos += seq_puts(s, "LOCK_INPUT,"); > + seq_puts(s, "LOCK_INPUT,"); > if (protect & DOC_PROTECT_STICKY_LOCK) > - pos += seq_puts(s, "STICKY_LOCK,"); > + seq_puts(s, "STICKY_LOCK,"); > if (protect & DOC_PROTECT_PROTECTION_ENABLED) > - pos += seq_puts(s, "PROTECTION ON,"); > + seq_puts(s, "PROTECTION ON,"); > if (protect & DOC_PROTECT_IPL_DOWNLOAD_LOCK) > - pos += seq_puts(s, "IPL_DOWNLOAD_LOCK,"); > + seq_puts(s, "IPL_DOWNLOAD_LOCK,"); > if (protect & DOC_PROTECT_PROTECTION_ERROR) > - pos += seq_puts(s, "PROTECT_ERR,"); > + seq_puts(s, "PROTECT_ERR,"); > else > - pos += seq_puts(s, "NO_PROTECT_ERR"); > - pos += seq_puts(s, ")\n"); > - > - pos += seq_printf(s, "DPS0 = 0x%02x : " > - "Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, " > - "WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n", > - dps0, dps0_low, dps0_high, > - !!(dps0 & DOC_DPS_OTP_PROTECTED), > - !!(dps0 & DOC_DPS_READ_PROTECTED), > - !!(dps0 & DOC_DPS_WRITE_PROTECTED), > - !!(dps0 & DOC_DPS_HW_LOCK_ENABLED), > - !!(dps0 & DOC_DPS_KEY_OK)); > - pos += seq_printf(s, "DPS1 = 0x%02x : " > - "Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, " > - "WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n", > - dps1, dps1_low, dps1_high, > - !!(dps1 & DOC_DPS_OTP_PROTECTED), > - !!(dps1 & DOC_DPS_READ_PROTECTED), > - !!(dps1 & DOC_DPS_WRITE_PROTECTED), > - !!(dps1 & DOC_DPS_HW_LOCK_ENABLED), > - !!(dps1 & DOC_DPS_KEY_OK)); > - return pos; > + seq_puts(s, "NO_PROTECT_ERR"); > + seq_puts(s, ")\n"); > + > + seq_printf(s, "DPS0 = 0x%02x : Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n", > + dps0, dps0_low, dps0_high, > + !!(dps0 & DOC_DPS_OTP_PROTECTED), > + !!(dps0 & DOC_DPS_READ_PROTECTED), > + !!(dps0 & DOC_DPS_WRITE_PROTECTED), > + !!(dps0 & DOC_DPS_HW_LOCK_ENABLED), > + !!(dps0 & DOC_DPS_KEY_OK)); > + seq_printf(s, "DPS1 = 0x%02x : Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n", > + dps1, dps1_low, dps1_high, > + !!(dps1 & DOC_DPS_OTP_PROTECTED), > + !!(dps1 & DOC_DPS_READ_PROTECTED), > + !!(dps1 & DOC_DPS_WRITE_PROTECTED), > + !!(dps1 & DOC_DPS_HW_LOCK_ENABLED), > + !!(dps1 & DOC_DPS_KEY_OK)); > + return 0; > } > DEBUGFS_RO_ATTR(protection, dbg_protection_show); > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 7/7] docg3: Fix miuse of seq_printf return value 2014-10-22 8:29 ` Brian Norris @ 2014-10-28 15:13 ` Steven Rostedt 2014-10-28 15:57 ` Joe Perches 0 siblings, 1 reply; 14+ messages in thread From: Steven Rostedt @ 2014-10-28 15:13 UTC (permalink / raw) To: Brian Norris Cc: Linus Torvalds, linux-kernel, Petr Mladek, linux-mtd, Al Viro, Joe Perches, Andrew Morton, David Woodhouse On Wed, 22 Oct 2014 01:29:16 -0700 Brian Norris <computersforpeace@gmail.com> wrote: > On Mon, Sep 29, 2014 at 04:08:27PM -0700, Joe Perches wrote: > > seq_printf doesn't return a useful value, so remove > > these misuses. > > Good catch. So it looks like this driver always had some form of > wrongness (returning a character count) in its debugfs callbacks, but > nobody noticed. > > Applied to l2-mtd.git. Thanks! > Note, I'm going to be working on changes to remove the return value of seq_printf() and friends. I need this change as well. If you already applied it to your git tree, if you can still rebase, can you make a separate branch off of Linus's tree that I can pull to do work on? Or I can take this patch as well, with your Acked-by. Thanks! -- Steve ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 7/7] docg3: Fix miuse of seq_printf return value 2014-10-28 15:13 ` Steven Rostedt @ 2014-10-28 15:57 ` Joe Perches 2014-10-28 16:05 ` Steven Rostedt 0 siblings, 1 reply; 14+ messages in thread From: Joe Perches @ 2014-10-28 15:57 UTC (permalink / raw) To: Steven Rostedt Cc: Linus Torvalds, linux-kernel, Petr Mladek, linux-mtd, Al Viro, Andrew Morton, Brian Norris, David Woodhouse On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote: > I'm going to be working on changes to remove the return value of > seq_printf() and friends. I'm sure you know all of this, but for anyone else: This doesn't need to be done in a single pass. The seq_<foo> functions themselves can wait until all the uses are converted. Here are files that likely need to change: $ git grep --name-only -E "[\(=]\s*seq_(vprintf|printf|puts|putc|escape|write|put_decimal_\w+)\s*\(" arch/arm/plat-pxa/dma.c arch/cris/arch-v10/kernel/fasttimer.c arch/cris/arch-v32/kernel/fasttimer.c arch/microblaze/kernel/cpu/mb.c arch/x86/kernel/cpu/mtrr/if.c drivers/base/power/wakeup.c drivers/mfd/ab8500-debugfs.c drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c drivers/parisc/ccio-dma.c drivers/parisc/sba_iommu.c drivers/regulator/dbx500-prcmu.c drivers/staging/lustre/lustre/fid/lproc_fid.c drivers/staging/lustre/lustre/llite/lproc_llite.c drivers/staging/lustre/lustre/mdc/lproc_mdc.c drivers/staging/lustre/lustre/obdclass/lprocfs_status.c drivers/staging/lustre/lustre/osc/lproc_osc.c drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c drivers/usb/gadget/udc/goku_udc.c drivers/usb/gadget/udc/pxa27x_udc.c drivers/watchdog/bcm_kona_wdt.c fs/debugfs/file.c fs/dlm/debug_fs.c fs/eventfd.c fs/eventpoll.c fs/notify/fdinfo.c fs/proc/base.c fs/seq_file.c kernel/trace/trace_seq.c net/batman-adv/gateway_client.c net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c net/netfilter/nf_conntrack_standalone.c net/netfilter/nf_log.c net/netfilter/xt_hashlimit.c ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 7/7] docg3: Fix miuse of seq_printf return value 2014-10-28 15:57 ` Joe Perches @ 2014-10-28 16:05 ` Steven Rostedt 2014-10-28 17:14 ` Brian Norris 2014-10-28 17:27 ` Joe Perches 0 siblings, 2 replies; 14+ messages in thread From: Steven Rostedt @ 2014-10-28 16:05 UTC (permalink / raw) To: Joe Perches Cc: Linus Torvalds, linux-kernel, Petr Mladek, linux-mtd, Al Viro, Andrew Morton, Brian Norris, David Woodhouse On Tue, 28 Oct 2014 08:57:57 -0700 Joe Perches <joe@perches.com> wrote: > On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote: > > I'm going to be working on changes to remove the return value of > > seq_printf() and friends. > > I'm sure you know all of this, but for anyone else: > > This doesn't need to be done in a single pass. Yeah, I took a look at all the places, and I've decided to take it off in chunks. I'm starting with the ones you posted, and will try to get acks for them. And then go after other chunks as I have time. I would like to get this done before I do my merge of trace_seq and seq_file, but I'm thinking I may have to do that in parallel. -- Steve ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 7/7] docg3: Fix miuse of seq_printf return value 2014-10-28 16:05 ` Steven Rostedt @ 2014-10-28 17:14 ` Brian Norris 2014-10-28 17:26 ` Steven Rostedt 2014-10-28 17:27 ` Joe Perches 1 sibling, 1 reply; 14+ messages in thread From: Brian Norris @ 2014-10-28 17:14 UTC (permalink / raw) To: Steven Rostedt Cc: Linus Torvalds, linux-kernel, Petr Mladek, linux-mtd, Al Viro, Joe Perches, Andrew Morton, David Woodhouse On Tue, Oct 28, 2014 at 12:05:52PM -0400, Steven Rostedt wrote: > On Tue, 28 Oct 2014 08:57:57 -0700 > Joe Perches <joe@perches.com> wrote: > > > On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote: > > > I'm going to be working on changes to remove the return value of > > > seq_printf() and friends. > > > > I'm sure you know all of this, but for anyone else: > > > > This doesn't need to be done in a single pass. > > Yeah, I took a look at all the places, and I've decided to take it off > in chunks. I'm starting with the ones you posted, and will try to get > acks for them. And then go after other chunks as I have time. I'll keep this in my tree as-is for now. Let me know if you still need something changed. I can give my 'Ack' if you really want to pull this into a separate branch. BTW, I just noticed the $subject has a typo: s/miuse/misuse/ Brian ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 7/7] docg3: Fix miuse of seq_printf return value 2014-10-28 17:14 ` Brian Norris @ 2014-10-28 17:26 ` Steven Rostedt 0 siblings, 0 replies; 14+ messages in thread From: Steven Rostedt @ 2014-10-28 17:26 UTC (permalink / raw) To: Brian Norris Cc: Linus Torvalds, linux-kernel, Petr Mladek, linux-mtd, Al Viro, Joe Perches, Andrew Morton, David Woodhouse On Tue, 28 Oct 2014 10:14:09 -0700 Brian Norris <computersforpeace@gmail.com> wrote: > On Tue, Oct 28, 2014 at 12:05:52PM -0400, Steven Rostedt wrote: > > On Tue, 28 Oct 2014 08:57:57 -0700 > > Joe Perches <joe@perches.com> wrote: > > > > > On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote: > > > > I'm going to be working on changes to remove the return value of > > > > seq_printf() and friends. > > > > > > I'm sure you know all of this, but for anyone else: > > > > > > This doesn't need to be done in a single pass. > > > > Yeah, I took a look at all the places, and I've decided to take it off > > in chunks. I'm starting with the ones you posted, and will try to get > > acks for them. And then go after other chunks as I have time. > > I'll keep this in my tree as-is for now. Let me know if you still need > something changed. I can give my 'Ack' if you really want to pull this > into a separate branch. If I get to a point where I can change the seq_printf() to return void, then I'll want it, otherwise without it, it will cause my branch to fail to compile on that code. That's why I would like it. If we keep it in your tree and have that for the next release, it will matter which tree goes into Linus's tree first. If mine goes in first, then it will break his build. Now, that's assuming I get this ready for 3.19, as I'll need quite a few Acked-by's for the changes that I'll be making. If I don't get it by 3.19, then it wont be an issue if that change goes in with your tree. -- Steve > > BTW, I just noticed the $subject has a typo: s/miuse/misuse/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 7/7] docg3: Fix miuse of seq_printf return value 2014-10-28 16:05 ` Steven Rostedt 2014-10-28 17:14 ` Brian Norris @ 2014-10-28 17:27 ` Joe Perches 2014-10-28 17:32 ` Steven Rostedt 1 sibling, 1 reply; 14+ messages in thread From: Joe Perches @ 2014-10-28 17:27 UTC (permalink / raw) To: Steven Rostedt Cc: Linus Torvalds, linux-kernel, Petr Mladek, linux-mtd, Al Viro, Andrew Morton, Brian Norris, David Woodhouse On Tue, 2014-10-28 at 12:05 -0400, Steven Rostedt wrote: > On Tue, 28 Oct 2014 08:57:57 -0700 > Joe Perches <joe@perches.com> wrote: > > > On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote: > > > I'm going to be working on changes to remove the return value of > > > seq_printf() and friends. > > > > I'm sure you know all of this, but for anyone else: > > > > This doesn't need to be done in a single pass. > > Yeah, I took a look at all the places, and I've decided to take it off > in chunks. I'm starting with the ones you posted, and will try to get > acks for them. And then go after other chunks as I have time. > > I would like to get this done before I do my merge of trace_seq and > seq_file, but I'm thinking I may have to do that in parallel. I think the most important thing is to get the seq_is_overflown (or seq_has_overflowed or whatever other name is chosen) function added then the rest of the patches can be applied whenever maintainers (or Andrew or trivial or ...) pick them up. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 7/7] docg3: Fix miuse of seq_printf return value 2014-10-28 17:27 ` Joe Perches @ 2014-10-28 17:32 ` Steven Rostedt 2014-10-28 17:36 ` Brian Norris 2014-10-28 17:38 ` Joe Perches 0 siblings, 2 replies; 14+ messages in thread From: Steven Rostedt @ 2014-10-28 17:32 UTC (permalink / raw) To: Joe Perches Cc: Linus Torvalds, linux-kernel, Petr Mladek, linux-mtd, Al Viro, Andrew Morton, Brian Norris, David Woodhouse On Tue, 28 Oct 2014 10:27:40 -0700 Joe Perches <joe@perches.com> wrote: > On Tue, 2014-10-28 at 12:05 -0400, Steven Rostedt wrote: > > On Tue, 28 Oct 2014 08:57:57 -0700 > > Joe Perches <joe@perches.com> wrote: > > > > > On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote: > > > > I'm going to be working on changes to remove the return value of > > > > seq_printf() and friends. > > > > > > I'm sure you know all of this, but for anyone else: > > > > > > This doesn't need to be done in a single pass. > > > > Yeah, I took a look at all the places, and I've decided to take it off > > in chunks. I'm starting with the ones you posted, and will try to get > > acks for them. And then go after other chunks as I have time. > > > > I would like to get this done before I do my merge of trace_seq and > > seq_file, but I'm thinking I may have to do that in parallel. > > I think the most important thing is to get the > seq_is_overflown (or seq_has_overflowed or whatever > other name is chosen) function added then the rest > of the patches can be applied whenever maintainers > (or Andrew or trivial or ...) pick them up. I can get that in without much of an issue. But the merge of trace_seq and seq_file would be easier if I didn't have to worry about return values. Which is why I want to get this in quickly. -- Steve ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 7/7] docg3: Fix miuse of seq_printf return value 2014-10-28 17:32 ` Steven Rostedt @ 2014-10-28 17:36 ` Brian Norris 2014-10-28 17:38 ` Joe Perches 1 sibling, 0 replies; 14+ messages in thread From: Brian Norris @ 2014-10-28 17:36 UTC (permalink / raw) To: Steven Rostedt Cc: Linus Torvalds, Linux Kernel, Petr Mladek, linux-mtd@lists.infradead.org, Al Viro, Joe Perches, Andrew Morton, David Woodhouse On Tue, Oct 28, 2014 at 10:32 AM, Steven Rostedt <rostedt@goodmis.org> wrote: > On Tue, 28 Oct 2014 10:27:40 -0700 > Joe Perches <joe@perches.com> wrote: >> On Tue, 2014-10-28 at 12:05 -0400, Steven Rostedt wrote: >> > On Tue, 28 Oct 2014 08:57:57 -0700 >> > Joe Perches <joe@perches.com> wrote: >> > >> > > On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote: >> > > > I'm going to be working on changes to remove the return value of >> > > > seq_printf() and friends. >> > > >> > > I'm sure you know all of this, but for anyone else: >> > > >> > > This doesn't need to be done in a single pass. >> > >> > Yeah, I took a look at all the places, and I've decided to take it off >> > in chunks. I'm starting with the ones you posted, and will try to get >> > acks for them. And then go after other chunks as I have time. >> > >> > I would like to get this done before I do my merge of trace_seq and >> > seq_file, but I'm thinking I may have to do that in parallel. >> >> I think the most important thing is to get the >> seq_is_overflown (or seq_has_overflowed or whatever >> other name is chosen) function added then the rest >> of the patches can be applied whenever maintainers >> (or Andrew or trivial or ...) pick them up. > > I can get that in without much of an issue. But the merge of trace_seq > and seq_file would be easier if I didn't have to worry about return > values. Which is why I want to get this in quickly. Whatever you'd like. Please, have my ack and keep the patch in your own branch! Acked-by: Brian Norris <computersforpeace@gmail.com> It's no problem if we both have the patch, is it? Or if I see it in linux-next, then I may drop it from my tree. Regards, Brian ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 7/7] docg3: Fix miuse of seq_printf return value 2014-10-28 17:32 ` Steven Rostedt 2014-10-28 17:36 ` Brian Norris @ 2014-10-28 17:38 ` Joe Perches 1 sibling, 0 replies; 14+ messages in thread From: Joe Perches @ 2014-10-28 17:38 UTC (permalink / raw) To: Steven Rostedt Cc: Linus Torvalds, linux-kernel, Petr Mladek, linux-mtd, Al Viro, Andrew Morton, Brian Norris, David Woodhouse On Tue, 2014-10-28 at 13:32 -0400, Steven Rostedt wrote: > On Tue, 28 Oct 2014 10:27:40 -0700 Joe Perches <joe@perches.com> wrote: > > On Tue, 2014-10-28 at 12:05 -0400, Steven Rostedt wrote: > > > I would like to get this done before I do my merge of trace_seq and > > > seq_file, but I'm thinking I may have to do that in parallel. > > > > I think the most important thing is to get the > > seq_is_overflown (or seq_has_overflowed or whatever > > other name is chosen) function added then the rest > > of the patches can be applied whenever maintainers > > (or Andrew or trivial or ...) pick them up. > > I can get that in without much of an issue. But the merge of trace_seq > and seq_file would be easier if I didn't have to worry about return > values. Which is why I want to get this in quickly. What is the issue there? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/7] seq_printf cleanups 2014-09-29 23:08 ` [PATCH 0/7] seq_printf cleanups Joe Perches 2014-09-29 23:08 ` [PATCH 7/7] docg3: Fix miuse of seq_printf return value Joe Perches @ 2014-10-28 15:32 ` Steven Rostedt 2014-10-28 15:51 ` Joe Perches 1 sibling, 1 reply; 14+ messages in thread From: Steven Rostedt @ 2014-10-28 15:32 UTC (permalink / raw) To: Joe Perches Cc: linux-doc, netdev, linux-kernel, Petr Mladek, cluster-devel, coreteam, linux-mtd, Al Viro, linux-fsdevel, netfilter-devel, Andrew Morton, Linus Torvalds Joe, If you haven't already done so, can you update checkpatch.pl to complain if someone checks the return value of seq_printf(), seq_puts(), or seq_putc(). It should state that those functions will soon be returning void. Thanks! -- Steve ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/7] seq_printf cleanups 2014-10-28 15:32 ` [PATCH 0/7] seq_printf cleanups Steven Rostedt @ 2014-10-28 15:51 ` Joe Perches 0 siblings, 0 replies; 14+ messages in thread From: Joe Perches @ 2014-10-28 15:51 UTC (permalink / raw) To: Steven Rostedt Cc: linux-doc, netdev, linux-kernel, Petr Mladek, cluster-devel, coreteam, linux-mtd, Al Viro, linux-fsdevel, netfilter-devel, Andrew Morton, Linus Torvalds On Tue, 2014-10-28 at 11:32 -0400, Steven Rostedt wrote: > If you haven't already done so, can you update checkpatch.pl to > complain if someone checks the return value of seq_printf(), > seq_puts(), or seq_putc(). I'm not sure that matters much as a rule because I hope soon the compiler will bleat when that happens. There are several more too: seq_vprintf seq_escape seq_write seq_bitmap seq_cpumask/seq_nodemask (and _list variants), seq_put_decimal_xxx ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-10-28 17:38 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20140929124246.3e39dac8@gandalf.local.home>
2014-09-29 23:08 ` [PATCH 0/7] seq_printf cleanups Joe Perches
2014-09-29 23:08 ` [PATCH 7/7] docg3: Fix miuse of seq_printf return value Joe Perches
2014-10-22 8:29 ` Brian Norris
2014-10-28 15:13 ` Steven Rostedt
2014-10-28 15:57 ` Joe Perches
2014-10-28 16:05 ` Steven Rostedt
2014-10-28 17:14 ` Brian Norris
2014-10-28 17:26 ` Steven Rostedt
2014-10-28 17:27 ` Joe Perches
2014-10-28 17:32 ` Steven Rostedt
2014-10-28 17:36 ` Brian Norris
2014-10-28 17:38 ` Joe Perches
2014-10-28 15:32 ` [PATCH 0/7] seq_printf cleanups Steven Rostedt
2014-10-28 15:51 ` Joe Perches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).