[PATCH] drivers/staging/batman-adv: Convert MAC_FMT to %pM

[PATCH] drivers/staging/batman-adv: Convert MAC_FMT to %pM

[Joe Perches]

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);
 }
 
 /* returns 1 if they are the same originator */
diff --git a/drivers/staging/batman-adv/translation-table.c b/drivers/staging/batman-adv/translation-table.c
index e01ff21..63d0967 100644
--- a/drivers/staging/batman-adv/translation-table.c
+++ b/drivers/staging/batman-adv/translation-table.c
@@ -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);

Re: [B.A.T.M.A.N.] [PATCH] drivers/staging/batman-adv: Convert MAC_FMT to %pM

[Sven Eckelmann]

[-- Attachment #1: Type: Text/Plain, Size: 1536 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] drivers/staging/batman-adv: Convert MAC_FMT to %pM

[Joe Perches]

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

Re: [B.A.T.M.A.N.] [PATCH] drivers/staging/batman-adv: Convert MAC_FMT to %pM

[Sven Eckelmann]

[-- Attachment #1: Type: Text/Plain, Size: 314 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] drivers/staging/batman-adv: Convert MAC_FMT to %pM

[Joe Perches]

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

Re: [B.A.T.M.A.N.] [PATCH] drivers/staging/batman-adv: Convert MAC_FMT to %pM

[Sven Eckelmann]

[-- Attachment #1: Type: Text/Plain, Size: 957 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] drivers/staging/batman-adv: Convert MAC_FMT to %pM

[Joe Perches]

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

Re: [B.A.T.M.A.N.] [PATCH] drivers/staging/batman-adv: Convert MAC_FMT to %pM

[Marek Lindner]

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

Re: [B.A.T.M.A.N.] [PATCH] drivers/staging/batman-adv: Convert MAC_FMT to %pM

[Greg KH]

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

Re: [B.A.T.M.A.N.] [PATCH] drivers/staging/batman-adv: Convert MAC_FMT to %pM

[Sven Eckelmann]

[-- Attachment #1: Type: Text/Plain, Size: 505 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]