* sysfs_format_mac @ 2013-07-16 19:56 David Miller 2013-07-16 20:46 ` sysfs_format_mac Joe Perches 0 siblings, 1 reply; 8+ messages in thread From: David Miller @ 2013-07-16 19:56 UTC (permalink / raw) To: netdev I assume that the only reason sysfs_format_mac() exists at all, and don't just use scnprintf() calls with "%pm\n", is that some devices have MAC addresses which are not 6 bytes in length. But it's such a waste to have this special case piece of code just for that. It would make so much more sense to allow specifying a length specifier to %pm and friends. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: sysfs_format_mac 2013-07-16 19:56 sysfs_format_mac David Miller @ 2013-07-16 20:46 ` Joe Perches 2013-07-16 20:58 ` sysfs_format_mac David Miller 0 siblings, 1 reply; 8+ messages in thread From: Joe Perches @ 2013-07-16 20:46 UTC (permalink / raw) To: David Miller; +Cc: netdev On Tue, 2013-07-16 at 12:56 -0700, David Miller wrote: > > I assume that the only reason sysfs_format_mac() exists at all, and > don't just use scnprintf() calls with "%pm\n", is that some devices > have MAC addresses which are not 6 bytes in length. > > But it's such a waste to have this special case piece of code > just for that. > > It would make so much more sense to allow specifying a length > specifier to %pm and friends. scnprintf("%*ph", (int)size, buffer) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: sysfs_format_mac 2013-07-16 20:46 ` sysfs_format_mac Joe Perches @ 2013-07-16 20:58 ` David Miller 2013-07-16 21:12 ` sysfs_format_mac Joe Perches 0 siblings, 1 reply; 8+ messages in thread From: David Miller @ 2013-07-16 20:58 UTC (permalink / raw) To: joe; +Cc: netdev From: Joe Perches <joe@perches.com> Date: Tue, 16 Jul 2013 13:46:26 -0700 > On Tue, 2013-07-16 at 12:56 -0700, David Miller wrote: >> >> I assume that the only reason sysfs_format_mac() exists at all, and >> don't just use scnprintf() calls with "%pm\n", is that some devices >> have MAC addresses which are not 6 bytes in length. >> >> But it's such a waste to have this special case piece of code >> just for that. >> >> It would make so much more sense to allow specifying a length >> specifier to %pm and friends. > > scnprintf("%*ph", (int)size, buffer) Yes, exactly, something like that. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: sysfs_format_mac 2013-07-16 20:58 ` sysfs_format_mac David Miller @ 2013-07-16 21:12 ` Joe Perches 2013-07-16 23:25 ` sysfs_format_mac David Miller 0 siblings, 1 reply; 8+ messages in thread From: Joe Perches @ 2013-07-16 21:12 UTC (permalink / raw) To: David Miller; +Cc: netdev On Tue, 2013-07-16 at 13:58 -0700, David Miller wrote: > From: Joe Perches <joe@perches.com> > Date: Tue, 16 Jul 2013 13:46:26 -0700 > > On Tue, 2013-07-16 at 12:56 -0700, David Miller wrote: [] > >> It would make so much more sense to allow specifying a length > >> specifier to %pm and friends. > > > > scnprintf("%*ph", (int)size, buffer) > > Yes, exactly, something like that. That works now. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: sysfs_format_mac 2013-07-16 21:12 ` sysfs_format_mac Joe Perches @ 2013-07-16 23:25 ` David Miller 2013-07-16 23:32 ` sysfs_format_mac Joe Perches 0 siblings, 1 reply; 8+ messages in thread From: David Miller @ 2013-07-16 23:25 UTC (permalink / raw) To: joe; +Cc: netdev From: Joe Perches <joe@perches.com> Date: Tue, 16 Jul 2013 14:12:49 -0700 > On Tue, 2013-07-16 at 13:58 -0700, David Miller wrote: >> From: Joe Perches <joe@perches.com> >> Date: Tue, 16 Jul 2013 13:46:26 -0700 >> > On Tue, 2013-07-16 at 12:56 -0700, David Miller wrote: > [] >> >> It would make so much more sense to allow specifying a length >> >> specifier to %pm and friends. >> > >> > scnprintf("%*ph", (int)size, buffer) >> >> Yes, exactly, something like that. > > That works now. Oh, hehe, indeed. I'm about to test the following. diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c index 5359560..2307062 100644 --- a/net/ethernet/eth.c +++ b/net/ethernet/eth.c @@ -401,27 +401,8 @@ struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs, } EXPORT_SYMBOL(alloc_etherdev_mqs); -static size_t _format_mac_addr(char *buf, int buflen, - const unsigned char *addr, int len) -{ - int i; - char *cp = buf; - - for (i = 0; i < len; i++) { - cp += scnprintf(cp, buflen - (cp - buf), "%02x", addr[i]); - if (i == len - 1) - break; - cp += scnprintf(cp, buflen - (cp - buf), ":"); - } - return cp - buf; -} - ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len) { - size_t l; - - l = _format_mac_addr(buf, PAGE_SIZE, addr, len); - l += scnprintf(buf + l, PAGE_SIZE - l, "\n"); - return (ssize_t)l; + return scnprintf(buf, PAGE_SIZE, "%*phC", len, addr); } EXPORT_SYMBOL(sysfs_format_mac); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: sysfs_format_mac 2013-07-16 23:25 ` sysfs_format_mac David Miller @ 2013-07-16 23:32 ` Joe Perches 2013-07-16 23:41 ` sysfs_format_mac David Miller 2013-07-17 0:10 ` sysfs_format_mac David Miller 0 siblings, 2 replies; 8+ messages in thread From: Joe Perches @ 2013-07-16 23:32 UTC (permalink / raw) To: David Miller; +Cc: netdev On Tue, 2013-07-16 at 16:25 -0700, David Miller wrote: > diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c [] > ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len) [] > - l = _format_mac_addr(buf, PAGE_SIZE, addr, len); > - l += scnprintf(buf + l, PAGE_SIZE - l, "\n"); [] > + return scnprintf(buf, PAGE_SIZE, "%*phC", len, addr); missing newline? scnprintf(buf, PAGE_ SIZE, "%*phC\n", len, addr); ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: sysfs_format_mac 2013-07-16 23:32 ` sysfs_format_mac Joe Perches @ 2013-07-16 23:41 ` David Miller 2013-07-17 0:10 ` sysfs_format_mac David Miller 1 sibling, 0 replies; 8+ messages in thread From: David Miller @ 2013-07-16 23:41 UTC (permalink / raw) To: joe; +Cc: netdev From: Joe Perches <joe@perches.com> Date: Tue, 16 Jul 2013 16:32:12 -0700 > On Tue, 2013-07-16 at 16:25 -0700, David Miller wrote: >> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c > [] >> ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len) > [] >> - l = _format_mac_addr(buf, PAGE_SIZE, addr, len); >> - l += scnprintf(buf + l, PAGE_SIZE - l, "\n"); > [] >> + return scnprintf(buf, PAGE_SIZE, "%*phC", len, addr); > > missing newline? > > scnprintf(buf, PAGE_ SIZE, "%*phC\n", len, addr); Good catch. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: sysfs_format_mac 2013-07-16 23:32 ` sysfs_format_mac Joe Perches 2013-07-16 23:41 ` sysfs_format_mac David Miller @ 2013-07-17 0:10 ` David Miller 1 sibling, 0 replies; 8+ messages in thread From: David Miller @ 2013-07-17 0:10 UTC (permalink / raw) To: joe; +Cc: netdev From: Joe Perches <joe@perches.com> Date: Tue, 16 Jul 2013 16:32:12 -0700 > On Tue, 2013-07-16 at 16:25 -0700, David Miller wrote: >> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c > [] >> ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len) > [] >> - l = _format_mac_addr(buf, PAGE_SIZE, addr, len); >> - l += scnprintf(buf + l, PAGE_SIZE - l, "\n"); > [] >> + return scnprintf(buf, PAGE_SIZE, "%*phC", len, addr); > > missing newline? > > scnprintf(buf, PAGE_ SIZE, "%*phC\n", len, addr); Thanks again Joe, I just commited the following patch: -------------------- [PATCH] net: Fix sysfs_format_mac() code duplication. It's just a duplicate implementation of "%*phC". Thanks to Joe Perches for showing that we had exactly this support in the lib/vsprintf.c code already. Signed-off-by: David S. Miller <davem@davemloft.net> --- net/ethernet/eth.c | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c index 5359560..be1f64d 100644 --- a/net/ethernet/eth.c +++ b/net/ethernet/eth.c @@ -401,27 +401,8 @@ struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs, } EXPORT_SYMBOL(alloc_etherdev_mqs); -static size_t _format_mac_addr(char *buf, int buflen, - const unsigned char *addr, int len) -{ - int i; - char *cp = buf; - - for (i = 0; i < len; i++) { - cp += scnprintf(cp, buflen - (cp - buf), "%02x", addr[i]); - if (i == len - 1) - break; - cp += scnprintf(cp, buflen - (cp - buf), ":"); - } - return cp - buf; -} - ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len) { - size_t l; - - l = _format_mac_addr(buf, PAGE_SIZE, addr, len); - l += scnprintf(buf + l, PAGE_SIZE - l, "\n"); - return (ssize_t)l; + return scnprintf(buf, PAGE_SIZE, "%*phC\n", len, addr); } EXPORT_SYMBOL(sysfs_format_mac); -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-07-17 0:10 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-16 19:56 sysfs_format_mac David Miller 2013-07-16 20:46 ` sysfs_format_mac Joe Perches 2013-07-16 20:58 ` sysfs_format_mac David Miller 2013-07-16 21:12 ` sysfs_format_mac Joe Perches 2013-07-16 23:25 ` sysfs_format_mac David Miller 2013-07-16 23:32 ` sysfs_format_mac Joe Perches 2013-07-16 23:41 ` sysfs_format_mac David Miller 2013-07-17 0:10 ` sysfs_format_mac David Miller
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).