* 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).