drivers/staging/batman-adv: Convert MAC_FMT to %pM
Commit Message
Remove the last uses of MAC_FMT
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/staging/batman-adv/main.c | 3 +-
drivers/staging/batman-adv/translation-table.c | 25 ++++-------------------
2 files changed, 6 insertions(+), 22 deletions(-)
Comments
Joe Perches wrote:
> Remove the last uses of MAC_FMT
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/staging/batman-adv/main.c | 3 +-
> drivers/staging/batman-adv/translation-table.c | 25
> ++++------------------- 2 files changed, 6 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/staging/batman-adv/main.c
> b/drivers/staging/batman-adv/main.c index 74c70d5..72851cd 100644
> --- a/drivers/staging/batman-adv/main.c
> +++ b/drivers/staging/batman-adv/main.c
> @@ -226,8 +226,7 @@ void dec_module_count(void)
>
> int addr_to_string(char *buff, uint8_t *addr)
> {
> - return sprintf(buff, MAC_FMT,
> - addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]);
> + return sprintf(buff, "%pM", addr);
> }
[...]
Thanks for your patch.
We must currently support older kernels which doesn't support %pM. Thats why
we have an extra wrapper for printk in the out-of-kernel module. The same
would have to be done for sprintf as well. Most of the developers are
currently attending the Wireless Battle Mesh v3 - so the processing of that
patch is delayed a little bit.
The same problem arises with your patch "Use (pr|netdev)_<level> macro helper"
(which seems to be added in 2.6.34). But I think we could add a wrapper for
older kernels easily - but as mentioned before this is probably postponed
until next week or so.
You've also changed the output. So you may partly broke batctl too (have to
check that first).
Best regards,
Sven
On Thu, 2010-06-03 at 01:23 +0200, Sven Eckelmann wrote:
> You've also changed the output. So you may partly broke batctl too (have to
> check that first).
There was no change in output.
MAC_FMT output is the same as %pM
cheers, Joe
Joe Perches wrote:
> On Thu, 2010-06-03 at 01:23 +0200, Sven Eckelmann wrote:
> > You've also changed the output. So you may partly broke batctl too (have
> > to check that first).
>
> There was no change in output.
> MAC_FMT output is the same as %pM
I meant the other patch :)
Best regards,
Sven
On Thu, 2010-06-03 at 01:33 +0200, Sven Eckelmann wrote:
> Joe Perches wrote:
> > On Thu, 2010-06-03 at 01:23 +0200, Sven Eckelmann wrote:
> > > You've also changed the output. So you may partly broke batctl too (have
> > > to check that first).
> > There was no change in output.
> > MAC_FMT output is the same as %pM
> I meant the other patch :)
batctl is parsing dmesg or equivalent? ouch.
If so, may I suggest you consider using something other than
a message logging parser for batctl?
I glanced at the source and don't see any such use.
It looks like it's only /sys, /proc and debugfs,
but I only spent a few seconds at it.
http://www.open-mesh.net/changeset/1682/trunk?old_path=%2F&format=zip
cheers, Joe
Joe Perches wrote:
> On Thu, 2010-06-03 at 01:33 +0200, Sven Eckelmann wrote:
> > Joe Perches wrote:
> > > On Thu, 2010-06-03 at 01:23 +0200, Sven Eckelmann wrote:
> > > > You've also changed the output. So you may partly broke batctl too
> > > > (have to check that first).
> > >
> > > There was no change in output.
> > > MAC_FMT output is the same as %pM
> >
> > I meant the other patch :)
>
> batctl is parsing dmesg or equivalent? ouch.
It has functionality to parse logfiles which for example could come from
dmesg.
> If so, may I suggest you consider using something other than
> a message logging parser for batctl?
>
> I glanced at the source and don't see any such use.
> It looks like it's only /sys, /proc and debugfs,
> but I only spent a few seconds at it.
>
> http://www.open-mesh.net/changeset/1682/trunk?old_path=%2F&format=zip
see sys.c -> log_print
and read_file -> read_file
Best regards,
Sven
On Thu, 2010-06-03 at 01:56 +0200, Sven Eckelmann wrote:
> see sys.c -> log_print
> and read_file -> read_file
Yuck.
The patch changes the prefix from "batman-adv:" to "batman_adv: "
so yes, it would break as-is.
I think the concept is broken though, I believe dmesg output
specifically is not guaranteed to remain stable, and batman should
use some other, perhaps private, logger based on ethtool events.
cheers, Joe
On Thursday 03 June 2010 08:20:01 Joe Perches wrote:
> The patch changes the prefix from "batman-adv:" to "batman_adv: "
> so yes, it would break as-is.
>
> I think the concept is broken though, I believe dmesg output
> specifically is not guaranteed to remain stable, and batman should
> use some other, perhaps private, logger based on ethtool events.
I think changing a dash to an underscore is not such a big deal (I did not
check the rest of the patch yet). But I'm interested to hear more about your
"private logger" idea because the current solution is far from being perfect.
As we have to debug the routing protocol every now and then it would be very
helpful to get direct access to some internal logging facility. In fact, that
existed before (inside of /proc) but was removed to be more compliant with the
linux kernel and the existing log facilities.
Regards,
Marek
Joe Perches wrote:
> Remove the last uses of MAC_FMT
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/staging/batman-adv/main.c | 3 +-
> drivers/staging/batman-adv/translation-table.c | 25
> ++++------------------- 2 files changed, 6 insertions(+), 22 deletions(-)
Just noticed that this patch collides with another one which is waiting in
GregKH's queue. I will try to rewrite it on top of that. I hope that this is
ok for you.
Best regards,
Sven
On Thu, Jun 03, 2010 at 05:15:53PM +0800, Marek Lindner wrote:
> On Thursday 03 June 2010 08:20:01 Joe Perches wrote:
> > The patch changes the prefix from "batman-adv:" to "batman_adv: "
> > so yes, it would break as-is.
> >
> > I think the concept is broken though, I believe dmesg output
> > specifically is not guaranteed to remain stable, and batman should
> > use some other, perhaps private, logger based on ethtool events.
>
> I think changing a dash to an underscore is not such a big deal (I did not
> check the rest of the patch yet). But I'm interested to hear more about your
> "private logger" idea because the current solution is far from being perfect.
> As we have to debug the routing protocol every now and then it would be very
> helpful to get direct access to some internal logging facility. In fact, that
> existed before (inside of /proc) but was removed to be more compliant with the
> linux kernel and the existing log facilities.
You can always use debugfs if you like for something like this.
Or tie into the profile/perf subsystem, that might be even easier.
thanks,
greg k-h
@@ -226,8 +226,7 @@ void dec_module_count(void)
int addr_to_string(char *buff, uint8_t *addr)
{
- return sprintf(buff, MAC_FMT,
- addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]);
+ return sprintf(buff, "%pM", addr);
}
/* returns 1 if they are the same originator */
@@ -202,13 +202,8 @@ int hna_local_fill_buffer_text(struct net_device *net_dev, char *buff,
hna_local_entry = hashit.bucket->data;
bytes_written += snprintf(buff + bytes_written, 22,
- " * " MAC_FMT "\n",
- hna_local_entry->addr[0],
- hna_local_entry->addr[1],
- hna_local_entry->addr[2],
- hna_local_entry->addr[3],
- hna_local_entry->addr[4],
- hna_local_entry->addr[5]);
+ " * %pM\n",
+ hna_local_entry->addr);
}
spin_unlock_irqrestore(&hna_local_hash_lock, flags);
@@ -420,19 +415,9 @@ int hna_global_fill_buffer_text(struct net_device *net_dev, char *buff,
hna_global_entry = hashit.bucket->data;
bytes_written += snprintf(buff + bytes_written, 44,
- " * " MAC_FMT " via " MAC_FMT "\n",
- hna_global_entry->addr[0],
- hna_global_entry->addr[1],
- hna_global_entry->addr[2],
- hna_global_entry->addr[3],
- hna_global_entry->addr[4],
- hna_global_entry->addr[5],
- hna_global_entry->orig_node->orig[0],
- hna_global_entry->orig_node->orig[1],
- hna_global_entry->orig_node->orig[2],
- hna_global_entry->orig_node->orig[3],
- hna_global_entry->orig_node->orig[4],
- hna_global_entry->orig_node->orig[5]);
+ " * %pM via %pM\n",
+ hna_global_entry->addr,
+ hna_global_entry->orig_node->orig);
}
spin_unlock_irqrestore(&hna_global_hash_lock, flags);