[B.A.T.M.A.N.] [resend][PATCHv2] staging: batman-adv: remove useless addr_to_string()

[B.A.T.M.A.N.] [resend][PATCHv2] staging: batman-adv: remove useless addr_to_string()

[Andy Shevchenko]

Since all *printf() methods in the kernel understand '%pM' modifier the
conversion to the string is useless beforehand.

Additionally this patch decreases batman_if structure by 20 bytes.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Marek Lindner <lindner_marek@yahoo.de>
Cc: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Cc: Andrew Lunn <andrew@lunn.ch>
---
 drivers/staging/batman-adv/hard-interface.c |    2 --
 drivers/staging/batman-adv/main.c           |    5 -----
 drivers/staging/batman-adv/main.h           |    2 --
 drivers/staging/batman-adv/originator.c     |   16 ++++++----------
 drivers/staging/batman-adv/routing.c        |    4 ++--
 drivers/staging/batman-adv/send.c           |    8 ++++----
 drivers/staging/batman-adv/types.h          |    1 -
 drivers/staging/batman-adv/vis.c            |   18 ++++++------------
 8 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/batman-adv/hard-interface.c b/drivers/staging/batman-adv/hard-interface.c
index 7b77cf2..80cfa86 100644
--- a/drivers/staging/batman-adv/hard-interface.c
+++ b/drivers/staging/batman-adv/hard-interface.c
@@ -159,8 +159,6 @@ static bool hardif_is_iface_up(struct batman_if *batman_if)
 
 static void update_mac_addresses(struct batman_if *batman_if)
 {
-	addr_to_string(batman_if->addr_str, batman_if->net_dev->dev_addr);
-
 	memcpy(((struct batman_packet *)(batman_if->packet_buff))->orig,
 	       batman_if->net_dev->dev_addr, ETH_ALEN);
 	memcpy(((struct batman_packet *)(batman_if->packet_buff))->prev_sender,
diff --git a/drivers/staging/batman-adv/main.c b/drivers/staging/batman-adv/main.c
index 580ca02..0587940 100644
--- a/drivers/staging/batman-adv/main.c
+++ b/drivers/staging/batman-adv/main.c
@@ -145,11 +145,6 @@ void dec_module_count(void)
 	module_put(THIS_MODULE);
 }
 
-int addr_to_string(char *buff, uint8_t *addr)
-{
-	return sprintf(buff, "%pM", addr);
-}
-
 /* returns 1 if they are the same originator */
 
 int compare_orig(void *data1, void *data2)
diff --git a/drivers/staging/batman-adv/main.h b/drivers/staging/batman-adv/main.h
index ca97589..5e3f516 100644
--- a/drivers/staging/batman-adv/main.h
+++ b/drivers/staging/batman-adv/main.h
@@ -58,7 +58,6 @@
 
 #define PACKBUFF_SIZE 2000
 #define LOG_BUF_LEN 8192	  /* has to be a power of 2 */
-#define ETH_STR_LEN 20
 
 #define VIS_INTERVAL 5000	/* 5 seconds */
 
@@ -136,7 +135,6 @@ int mesh_init(struct net_device *soft_iface);
 void mesh_free(struct net_device *soft_iface);
 void inc_module_count(void);
 void dec_module_count(void);
-int addr_to_string(char *buff, uint8_t *addr);
 int compare_orig(void *data1, void *data2);
 int choose_orig(void *data, int32_t size);
 int is_my_mac(uint8_t *addr);
diff --git a/drivers/staging/batman-adv/originator.c b/drivers/staging/batman-adv/originator.c
index 865211d..5527008 100644
--- a/drivers/staging/batman-adv/originator.c
+++ b/drivers/staging/batman-adv/originator.c
@@ -315,7 +315,6 @@ int orig_seq_print_text(struct seq_file *seq, void *offset)
 	int last_seen_secs;
 	int last_seen_msecs;
 	unsigned long flags;
-	char orig_str[ETH_STR_LEN], router_str[ETH_STR_LEN];
 
 	if ((!bat_priv->primary_if) ||
 	    (bat_priv->primary_if->if_status != IF_ACTIVE)) {
@@ -329,10 +328,10 @@ int orig_seq_print_text(struct seq_file *seq, void *offset)
 				  net_dev->name);
 	}
 
-	seq_printf(seq, "[B.A.T.M.A.N. adv %s%s, MainIF/MAC: %s/%s (%s)]\n",
+	seq_printf(seq, "[B.A.T.M.A.N. adv %s%s, MainIF/MAC: %s/%pM (%s)]\n",
 		   SOURCE_VERSION, REVISION_VERSION_STR,
 		   bat_priv->primary_if->net_dev->name,
-		   bat_priv->primary_if->addr_str, net_dev->name);
+		   bat_priv->primary_if->net_dev->dev_addr, net_dev->name);
 	seq_printf(seq, "  %-15s %s (%s/%i) %17s [%10s]: %20s ...\n",
 		   "Originator", "last-seen", "#", TQ_MAX_VALUE, "Nexthop",
 		   "outgoingIF", "Potential nexthops");
@@ -349,21 +348,18 @@ int orig_seq_print_text(struct seq_file *seq, void *offset)
 		if (orig_node->router->tq_avg == 0)
 			continue;
 
-		addr_to_string(orig_str, orig_node->orig);
-		addr_to_string(router_str, orig_node->router->addr);
 		last_seen_secs = jiffies_to_msecs(jiffies -
 						orig_node->last_valid) / 1000;
 		last_seen_msecs = jiffies_to_msecs(jiffies -
 						orig_node->last_valid) % 1000;
 
-		seq_printf(seq, "%-17s %4i.%03is   (%3i) %17s [%10s]:",
-			   orig_str, last_seen_secs, last_seen_msecs,
-			   orig_node->router->tq_avg, router_str,
+		seq_printf(seq, "%pM %4i.%03is   (%3i) %pM [%10s]:",
+			   orig_node->orig, last_seen_secs, last_seen_msecs,
+			   orig_node->router->tq_avg, orig_node->router->addr,
 			   orig_node->router->if_incoming->net_dev->name);
 
 		list_for_each_entry(neigh_node, &orig_node->neigh_list, list) {
-			addr_to_string(orig_str, neigh_node->addr);
-			seq_printf(seq, " %17s (%3i)", orig_str,
+			seq_printf(seq, " %pM (%3i)", neigh_node->addr,
 					   neigh_node->tq_avg);
 		}
 
diff --git a/drivers/staging/batman-adv/routing.c b/drivers/staging/batman-adv/routing.c
index 58aa99e..9010263 100644
--- a/drivers/staging/batman-adv/routing.c
+++ b/drivers/staging/batman-adv/routing.c
@@ -554,11 +554,11 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 					    batman_packet->orig) ? 1 : 0);
 
 	bat_dbg(DBG_BATMAN, bat_priv,
-		"Received BATMAN packet via NB: %pM, IF: %s [%s] "
+		"Received BATMAN packet via NB: %pM, IF: %s [%pM] "
 		"(from OG: %pM, via prev OG: %pM, seqno %d, tq %d, "
 		"TTL %d, V %d, IDF %d)\n",
 		ethhdr->h_source, if_incoming->net_dev->name,
-		if_incoming->addr_str, batman_packet->orig,
+		if_incoming->net_dev->dev_addr, batman_packet->orig,
 		batman_packet->prev_sender, batman_packet->seqno,
 		batman_packet->tq, batman_packet->ttl, batman_packet->version,
 		has_directlink_flag);
diff --git a/drivers/staging/batman-adv/send.c b/drivers/staging/batman-adv/send.c
index 9032861..7adf76d 100644
--- a/drivers/staging/batman-adv/send.c
+++ b/drivers/staging/batman-adv/send.c
@@ -135,13 +135,13 @@ static void send_packet_to_if(struct forw_packet *forw_packet,
 							    "Forwarding"));
 		bat_dbg(DBG_BATMAN, bat_priv,
 			"%s %spacket (originator %pM, seqno %d, TQ %d, TTL %d,"
-			" IDF %s) on interface %s [%s]\n",
+			" IDF %s) on interface %s [%pM]\n",
 			fwd_str, (packet_num > 0 ? "aggregated " : ""),
 			batman_packet->orig, ntohl(batman_packet->seqno),
 			batman_packet->tq, batman_packet->ttl,
 			(batman_packet->flags & DIRECTLINK ?
 			 "on" : "off"),
-			batman_if->net_dev->name, batman_if->addr_str);
+			batman_if->net_dev->name, batman_if->net_dev->dev_addr);
 
 		buff_pos += sizeof(struct batman_packet) +
 			(batman_packet->num_hna * ETH_ALEN);
@@ -186,12 +186,12 @@ static void send_packet(struct forw_packet *forw_packet)
 		/* FIXME: what about aggregated packets ? */
 		bat_dbg(DBG_BATMAN, bat_priv,
 			"%s packet (originator %pM, seqno %d, TTL %d) "
-			"on interface %s [%s]\n",
+			"on interface %s [%pM]\n",
 			(forw_packet->own ? "Sending own" : "Forwarding"),
 			batman_packet->orig, ntohl(batman_packet->seqno),
 			batman_packet->ttl,
 			forw_packet->if_incoming->net_dev->name,
-			forw_packet->if_incoming->addr_str);
+			forw_packet->if_incoming->net_dev->dev_addr);
 
 		/* skb is only used once and than forw_packet is free'd */
 		send_skb_packet(forw_packet->skb, forw_packet->if_incoming,
diff --git a/drivers/staging/batman-adv/types.h b/drivers/staging/batman-adv/types.h
index bb5827f..f3f7366 100644
--- a/drivers/staging/batman-adv/types.h
+++ b/drivers/staging/batman-adv/types.h
@@ -37,7 +37,6 @@ struct batman_if {
 	struct list_head list;
 	int16_t if_num;
 	char if_status;
-	char addr_str[ETH_STR_LEN];
 	struct net_device *net_dev;
 	atomic_t seqno;
 	atomic_t frag_seqno;
diff --git a/drivers/staging/batman-adv/vis.c b/drivers/staging/batman-adv/vis.c
index 6b102a3..3d2c1bc 100644
--- a/drivers/staging/batman-adv/vis.c
+++ b/drivers/staging/batman-adv/vis.c
@@ -130,15 +130,13 @@ static ssize_t vis_data_read_prim_sec(char *buff, struct hlist_head *if_list)
 {
 	struct if_list_entry *entry;
 	struct hlist_node *pos;
-	char tmp_addr_str[ETH_STR_LEN];
 	size_t len = 0;
 
 	hlist_for_each_entry(entry, pos, if_list, list) {
 		if (entry->primary)
 			len += sprintf(buff + len, "PRIMARY, ");
 		else {
-			addr_to_string(tmp_addr_str, entry->addr);
-			len += sprintf(buff + len,  "SEC %s, ", tmp_addr_str);
+			len += sprintf(buff + len,  "SEC %pM, ", entry->addr);
 		}
 	}
 
@@ -165,14 +163,12 @@ static size_t vis_data_count_prim_sec(struct hlist_head *if_list)
 static ssize_t vis_data_read_entry(char *buff, struct vis_info_entry *entry,
 				   uint8_t *src, bool primary)
 {
-	char to[18];
-
 	/* maximal length: max(4+17+2, 3+17+1+3+2) == 26 */
-	addr_to_string(to, entry->dest);
 	if (primary && entry->quality == 0)
-		return sprintf(buff, "HNA %s, ", to);
+		return sprintf(buff, "HNA %pM, ", entry->dest);
 	else if (compare_orig(entry->src, src))
-		return sprintf(buff, "TQ %s %d, ", to, entry->quality);
+		return sprintf(buff, "TQ %pM %d, ", entry->dest,
+			       entry->quality);
 
 	return 0;
 }
@@ -190,7 +186,6 @@ int vis_seq_print_text(struct seq_file *seq, void *offset)
 	struct if_list_entry *entry;
 	struct hlist_node *pos, *n;
 	int i;
-	char tmp_addr_str[ETH_STR_LEN];
 	unsigned long flags;
 	int vis_server = atomic_read(&bat_priv->vis_mode);
 	size_t buff_pos, buf_size;
@@ -255,9 +250,8 @@ int vis_seq_print_text(struct seq_file *seq, void *offset)
 		}
 
 		hlist_for_each_entry(entry, pos, &vis_if_list, list) {
-			addr_to_string(tmp_addr_str, entry->addr);
-			buff_pos += sprintf(buff + buff_pos, "%s,",
-					    tmp_addr_str);
+			buff_pos += sprintf(buff + buff_pos, "%pM,",
+					    entry->addr);
 
 			for (i = 0; i < packet->entries; i++)
 				buff_pos += vis_data_read_entry(buff + buff_pos,
-- 
1.7.2.3

Re: [B.A.T.M.A.N.] [resend][PATCHv2] staging: batman-adv: remove useless addr_to_string()

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 437 bytes --]

On Wed, Oct 20, 2010 at 06:47:44PM +0300, Andy Shevchenko wrote:
> Since all *printf() methods in the kernel understand '%pM' modifier the
> conversion to the string is useless beforehand.
> 
> Additionally this patch decreases batman_if structure by 20 bytes.

Thanks for your patch. I have problems with compiling due to other patches in
the queue. I will fix that and recommend it as patch for 2.6.38.

Best regards,
	Sven

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] [resend][PATCHv2] staging: batman-adv: remove useless addr_to_string()

[Greg KH]

On Wed, Oct 20, 2010 at 10:51:15PM +0200, Sven Eckelmann wrote:
> On Wed, Oct 20, 2010 at 06:47:44PM +0300, Andy Shevchenko wrote:
> > Since all *printf() methods in the kernel understand '%pM' modifier the
> > conversion to the string is useless beforehand.
> > 
> > Additionally this patch decreases batman_if structure by 20 bytes.
> 
> Thanks for your patch. I have problems with compiling due to other patches in
> the queue. I will fix that and recommend it as patch for 2.6.38.

What do you mean by this?  It applies just fine to my tree, so why can't
I take it now?

thanks,

greg k-h

[B.A.T.M.A.N.] [PATCHv3] batman-adv: remove useless addr_to_string()

[Sven Eckelmann]

From: Andy Shevchenko <andy.shevchenko@gmail.com>

Since all *printf() methods in the kernel understand '%pM' modifier the
conversion to the string is useless beforehand.

Additionally this patch decreases batman_if structure by 20 bytes.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
[sven.eckelmann@gmx.de: Fix compilation errors]
Signed-off-by: Sven Eckelmann <sven.eckelmann@gmx.de>
---
 batman-adv/gateway_client.c |   13 +++++--------
 batman-adv/hard-interface.c |    2 --
 batman-adv/main.c           |    5 -----
 batman-adv/main.h           |    2 --
 batman-adv/originator.c     |   16 ++++++----------
 batman-adv/routing.c        |    4 ++--
 batman-adv/send.c           |    8 ++++----
 batman-adv/types.h          |    1 -
 batman-adv/vis.c            |   18 ++++++------------
 9 files changed, 23 insertions(+), 46 deletions(-)

diff --git a/batman-adv/gateway_client.c b/batman-adv/gateway_client.c
index ae1ab1e..77f1044 100644
--- a/batman-adv/gateway_client.c
+++ b/batman-adv/gateway_client.c
@@ -351,17 +351,14 @@ static int _write_buffer_text(struct bat_priv *bat_priv,
 			      struct seq_file *seq, struct gw_node *gw_node)
 {
 	int down, up;
-	char gw_str[ETH_STR_LEN], router_str[ETH_STR_LEN];
 
-	addr_to_string(gw_str, gw_node->orig_node->orig);
-	addr_to_string(router_str, gw_node->orig_node->router->addr);
 	gw_srv_class_to_kbit(gw_node->orig_node->gw_flags, &down, &up);
 
-	return seq_printf(seq, "%s %-17s (%3i) %17s [%10s]: %3i - %i%s/%i%s\n",
+	return seq_printf(seq, "%s %pM (%3i) %pM [%10s]: %3i - %i%s/%i%s\n",
 		       (bat_priv->curr_gw == gw_node ? "=>" : "  "),
-		       gw_str,
+		       gw_node->orig_node->orig,
 		       gw_node->orig_node->router->tq_avg,
-		       router_str,
+		       gw_node->orig_node->router->addr,
 		       gw_node->orig_node->router->if_incoming->net_dev->name,
 		       gw_node->orig_node->gw_flags,
 		       (down > 2048 ? down / 1024 : down),
@@ -393,11 +390,11 @@ int gw_client_seq_print_text(struct seq_file *seq, void *offset)
 	}
 
 	seq_printf(seq, "      %-12s (%s/%i) %17s [%10s]: gw_class ... "
-		   "[B.A.T.M.A.N. adv %s%s, MainIF/MAC: %s/%s (%s)]\n",
+		   "[B.A.T.M.A.N. adv %s%s, MainIF/MAC: %s/%pM (%s)]\n",
 		   "Gateway", "#", TQ_MAX_VALUE, "Nexthop",
 		   "outgoingIF", SOURCE_VERSION, REVISION_VERSION_STR,
 		   bat_priv->primary_if->net_dev->name,
-		   bat_priv->primary_if->addr_str, net_dev->name);
+		   bat_priv->primary_if->net_dev->dev_addr, net_dev->name);
 
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(gw_node, node, &bat_priv->gw_list, list) {
diff --git a/batman-adv/hard-interface.c b/batman-adv/hard-interface.c
index 3c59209..ef95680 100644
--- a/batman-adv/hard-interface.c
+++ b/batman-adv/hard-interface.c
@@ -171,8 +171,6 @@ static bool hardif_is_iface_up(struct batman_if *batman_if)
 
 static void update_mac_addresses(struct batman_if *batman_if)
 {
-	addr_to_string(batman_if->addr_str, batman_if->net_dev->dev_addr);
-
 	memcpy(((struct batman_packet *)(batman_if->packet_buff))->orig,
 	       batman_if->net_dev->dev_addr, ETH_ALEN);
 	memcpy(((struct batman_packet *)(batman_if->packet_buff))->prev_sender,
diff --git a/batman-adv/main.c b/batman-adv/main.c
index 6ecee49..b932c4d 100644
--- a/batman-adv/main.c
+++ b/batman-adv/main.c
@@ -154,11 +154,6 @@ void dec_module_count(void)
 	module_put(THIS_MODULE);
 }
 
-int addr_to_string(char *buff, uint8_t *addr)
-{
-	return sprintf(buff, "%pM", addr);
-}
-
 /* returns 1 if they are the same originator */
 
 int compare_orig(void *data1, void *data2)
diff --git a/batman-adv/main.h b/batman-adv/main.h
index cc42eb4..a4337ea 100644
--- a/batman-adv/main.h
+++ b/batman-adv/main.h
@@ -58,7 +58,6 @@
 
 #define PACKBUFF_SIZE 2000
 #define LOG_BUF_LEN 8192	  /* has to be a power of 2 */
-#define ETH_STR_LEN 20
 
 #define VIS_INTERVAL 5000	/* 5 seconds */
 
@@ -138,7 +137,6 @@ int mesh_init(struct net_device *soft_iface);
 void mesh_free(struct net_device *soft_iface);
 void inc_module_count(void);
 void dec_module_count(void);
-int addr_to_string(char *buff, uint8_t *addr);
 int compare_orig(void *data1, void *data2);
 int choose_orig(void *data, int32_t size);
 int is_my_mac(uint8_t *addr);
diff --git a/batman-adv/originator.c b/batman-adv/originator.c
index 6868a2a..db2efc2 100644
--- a/batman-adv/originator.c
+++ b/batman-adv/originator.c
@@ -324,7 +324,6 @@ int orig_seq_print_text(struct seq_file *seq, void *offset)
 	int last_seen_secs;
 	int last_seen_msecs;
 	unsigned long flags;
-	char orig_str[ETH_STR_LEN], router_str[ETH_STR_LEN];
 
 	if ((!bat_priv->primary_if) ||
 	    (bat_priv->primary_if->if_status != IF_ACTIVE)) {
@@ -338,10 +337,10 @@ int orig_seq_print_text(struct seq_file *seq, void *offset)
 				  net_dev->name);
 	}
 
-	seq_printf(seq, "[B.A.T.M.A.N. adv %s%s, MainIF/MAC: %s/%s (%s)]\n",
+	seq_printf(seq, "[B.A.T.M.A.N. adv %s%s, MainIF/MAC: %s/%pM (%s)]\n",
 		   SOURCE_VERSION, REVISION_VERSION_STR,
 		   bat_priv->primary_if->net_dev->name,
-		   bat_priv->primary_if->addr_str, net_dev->name);
+		   bat_priv->primary_if->net_dev->dev_addr, net_dev->name);
 	seq_printf(seq, "  %-15s %s (%s/%i) %17s [%10s]: %20s ...\n",
 		   "Originator", "last-seen", "#", TQ_MAX_VALUE, "Nexthop",
 		   "outgoingIF", "Potential nexthops");
@@ -358,21 +357,18 @@ int orig_seq_print_text(struct seq_file *seq, void *offset)
 		if (orig_node->router->tq_avg == 0)
 			continue;
 
-		addr_to_string(orig_str, orig_node->orig);
-		addr_to_string(router_str, orig_node->router->addr);
 		last_seen_secs = jiffies_to_msecs(jiffies -
 						orig_node->last_valid) / 1000;
 		last_seen_msecs = jiffies_to_msecs(jiffies -
 						orig_node->last_valid) % 1000;
 
-		seq_printf(seq, "%-17s %4i.%03is   (%3i) %17s [%10s]:",
-			   orig_str, last_seen_secs, last_seen_msecs,
-			   orig_node->router->tq_avg, router_str,
+		seq_printf(seq, "%pM %4i.%03is   (%3i) %pM [%10s]:",
+			   orig_node->orig, last_seen_secs, last_seen_msecs,
+			   orig_node->router->tq_avg, orig_node->router->addr,
 			   orig_node->router->if_incoming->net_dev->name);
 
 		list_for_each_entry(neigh_node, &orig_node->neigh_list, list) {
-			addr_to_string(orig_str, neigh_node->addr);
-			seq_printf(seq, " %17s (%3i)", orig_str,
+			seq_printf(seq, " %pM (%3i)", neigh_node->addr,
 					   neigh_node->tq_avg);
 		}
 
diff --git a/batman-adv/routing.c b/batman-adv/routing.c
index cc8b77f..e28fffe 100644
--- a/batman-adv/routing.c
+++ b/batman-adv/routing.c
@@ -566,11 +566,11 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 					    batman_packet->orig) ? 1 : 0);
 
 	bat_dbg(DBG_BATMAN, bat_priv,
-		"Received BATMAN packet via NB: %pM, IF: %s [%s] "
+		"Received BATMAN packet via NB: %pM, IF: %s [%pM] "
 		"(from OG: %pM, via prev OG: %pM, seqno %d, tq %d, "
 		"TTL %d, V %d, IDF %d)\n",
 		ethhdr->h_source, if_incoming->net_dev->name,
-		if_incoming->addr_str, batman_packet->orig,
+		if_incoming->net_dev->dev_addr, batman_packet->orig,
 		batman_packet->prev_sender, batman_packet->seqno,
 		batman_packet->tq, batman_packet->ttl, batman_packet->version,
 		has_directlink_flag);
diff --git a/batman-adv/send.c b/batman-adv/send.c
index 180e18b..cfacb4d 100644
--- a/batman-adv/send.c
+++ b/batman-adv/send.c
@@ -137,13 +137,13 @@ static void send_packet_to_if(struct forw_packet *forw_packet,
 							    "Forwarding"));
 		bat_dbg(DBG_BATMAN, bat_priv,
 			"%s %spacket (originator %pM, seqno %d, TQ %d, TTL %d,"
-			" IDF %s) on interface %s [%s]\n",
+			" IDF %s) on interface %s [%pM]\n",
 			fwd_str, (packet_num > 0 ? "aggregated " : ""),
 			batman_packet->orig, ntohl(batman_packet->seqno),
 			batman_packet->tq, batman_packet->ttl,
 			(batman_packet->flags & DIRECTLINK ?
 			 "on" : "off"),
-			batman_if->net_dev->name, batman_if->addr_str);
+			batman_if->net_dev->name, batman_if->net_dev->dev_addr);
 
 		buff_pos += sizeof(struct batman_packet) +
 			(batman_packet->num_hna * ETH_ALEN);
@@ -188,12 +188,12 @@ static void send_packet(struct forw_packet *forw_packet)
 		/* FIXME: what about aggregated packets ? */
 		bat_dbg(DBG_BATMAN, bat_priv,
 			"%s packet (originator %pM, seqno %d, TTL %d) "
-			"on interface %s [%s]\n",
+			"on interface %s [%pM]\n",
 			(forw_packet->own ? "Sending own" : "Forwarding"),
 			batman_packet->orig, ntohl(batman_packet->seqno),
 			batman_packet->ttl,
 			forw_packet->if_incoming->net_dev->name,
-			forw_packet->if_incoming->addr_str);
+			forw_packet->if_incoming->net_dev->dev_addr);
 
 		/* skb is only used once and than forw_packet is free'd */
 		send_skb_packet(forw_packet->skb, forw_packet->if_incoming,
diff --git a/batman-adv/types.h b/batman-adv/types.h
index 44b3c07..b6e5a88 100644
--- a/batman-adv/types.h
+++ b/batman-adv/types.h
@@ -37,7 +37,6 @@ struct batman_if {
 	struct list_head list;
 	int16_t if_num;
 	char if_status;
-	char addr_str[ETH_STR_LEN];
 	struct net_device *net_dev;
 	atomic_t seqno;
 	atomic_t frag_seqno;
diff --git a/batman-adv/vis.c b/batman-adv/vis.c
index ddfdcd8..ebd85b3 100644
--- a/batman-adv/vis.c
+++ b/batman-adv/vis.c
@@ -131,15 +131,13 @@ static ssize_t vis_data_read_prim_sec(char *buff, struct hlist_head *if_list)
 {
 	struct if_list_entry *entry;
 	struct hlist_node *pos;
-	char tmp_addr_str[ETH_STR_LEN];
 	size_t len = 0;
 
 	hlist_for_each_entry(entry, pos, if_list, list) {
 		if (entry->primary)
 			len += sprintf(buff + len, "PRIMARY, ");
 		else {
-			addr_to_string(tmp_addr_str, entry->addr);
-			len += sprintf(buff + len,  "SEC %s, ", tmp_addr_str);
+			len += sprintf(buff + len,  "SEC %pM, ", entry->addr);
 		}
 	}
 
@@ -166,14 +164,12 @@ static size_t vis_data_count_prim_sec(struct hlist_head *if_list)
 static ssize_t vis_data_read_entry(char *buff, struct vis_info_entry *entry,
 				   uint8_t *src, bool primary)
 {
-	char to[18];
-
 	/* maximal length: max(4+17+2, 3+17+1+3+2) == 26 */
-	addr_to_string(to, entry->dest);
 	if (primary && entry->quality == 0)
-		return sprintf(buff, "HNA %s, ", to);
+		return sprintf(buff, "HNA %pM, ", entry->dest);
 	else if (compare_orig(entry->src, src))
-		return sprintf(buff, "TQ %s %d, ", to, entry->quality);
+		return sprintf(buff, "TQ %pM %d, ", entry->dest,
+			       entry->quality);
 
 	return 0;
 }
@@ -191,7 +187,6 @@ int vis_seq_print_text(struct seq_file *seq, void *offset)
 	struct if_list_entry *entry;
 	struct hlist_node *pos, *n;
 	int i;
-	char tmp_addr_str[ETH_STR_LEN];
 	unsigned long flags;
 	int vis_server = atomic_read(&bat_priv->vis_mode);
 	size_t buff_pos, buf_size;
@@ -256,9 +251,8 @@ int vis_seq_print_text(struct seq_file *seq, void *offset)
 		}
 
 		hlist_for_each_entry(entry, pos, &vis_if_list, list) {
-			addr_to_string(tmp_addr_str, entry->addr);
-			buff_pos += sprintf(buff + buff_pos, "%s,",
-					    tmp_addr_str);
+			buff_pos += sprintf(buff + buff_pos, "%pM,",
+					    entry->addr);
 
 			for (i = 0; i < packet->entries; i++)
 				buff_pos += vis_data_read_entry(buff + buff_pos,
-- 
1.7.1

Re: [B.A.T.M.A.N.] [resend][PATCHv2] staging: batman-adv: remove useless addr_to_string()

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

On Wed, Oct 20, 2010 at 01:57:49PM -0700, Greg KH wrote:
> On Wed, Oct 20, 2010 at 10:51:15PM +0200, Sven Eckelmann wrote:
> > On Wed, Oct 20, 2010 at 06:47:44PM +0300, Andy Shevchenko wrote:
> > > Since all *printf() methods in the kernel understand '%pM' modifier the
> > > conversion to the string is useless beforehand.
> > > 
> > > Additionally this patch decreases batman_if structure by 20 bytes.
> > 
> > Thanks for your patch. I have problems with compiling due to other patches in
> > the queue. I will fix that and recommend it as patch for 2.6.38.
> 
> What do you mean by this?  It applies just fine to my tree, so why can't
> I take it now?

If you want then do so, but the stuff in batman-adv's master must be fixed so
they have to apply the v3 version of the patch and not the v2 version Andy
sent.

Best regards,
	Sven

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] [resend][PATCHv2] staging: batman-adv: remove useless addr_to_string()

[Greg KH]

On Thu, Oct 21, 2010 at 12:16:37AM +0200, Sven Eckelmann wrote:
> On Wed, Oct 20, 2010 at 01:57:49PM -0700, Greg KH wrote:
> > On Wed, Oct 20, 2010 at 10:51:15PM +0200, Sven Eckelmann wrote:
> > > On Wed, Oct 20, 2010 at 06:47:44PM +0300, Andy Shevchenko wrote:
> > > > Since all *printf() methods in the kernel understand '%pM' modifier the
> > > > conversion to the string is useless beforehand.
> > > > 
> > > > Additionally this patch decreases batman_if structure by 20 bytes.
> > > 
> > > Thanks for your patch. I have problems with compiling due to other patches in
> > > the queue. I will fix that and recommend it as patch for 2.6.38.
> > 
> > What do you mean by this?  It applies just fine to my tree, so why can't
> > I take it now?
> 
> If you want then do so, but the stuff in batman-adv's master must be fixed so
> they have to apply the v3 version of the patch and not the v2 version Andy
> sent.

That's one of the problems with having an out-of-tree tree.  Please
don't do that at all anymore.

I'll go apply this patch to mine, and you can handle any merge issues if
you continue to wish to keep an external tree (hint, I STRONGLY
recommend that you do not, for these reasons and many others.)

thanks,

greg k-h

Re: [B.A.T.M.A.N.] [resend][PATCHv2] staging: batman-adv: remove useless addr_to_string()

[Sven Eckelmann]

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

Greg KH wrote:
> On Thu, Oct 21, 2010 at 12:16:37AM +0200, Sven Eckelmann wrote:
> > On Wed, Oct 20, 2010 at 01:57:49PM -0700, Greg KH wrote:
> > > On Wed, Oct 20, 2010 at 10:51:15PM +0200, Sven Eckelmann wrote:
> > > > On Wed, Oct 20, 2010 at 06:47:44PM +0300, Andy Shevchenko wrote:
> > > > > Since all *printf() methods in the kernel understand '%pM' modifier
> > > > > the conversion to the string is useless beforehand.
> > > > > 
> > > > > Additionally this patch decreases batman_if structure by 20 bytes.
> > > > 
> > > > Thanks for your patch. I have problems with compiling due to other
> > > > patches in the queue. I will fix that and recommend it as patch for
> > > > 2.6.38.
> > > 
> > > What do you mean by this?  It applies just fine to my tree, so why
> > > can't I take it now?
> > 
> > If you want then do so, but the stuff in batman-adv's master must be
> > fixed so they have to apply the v3 version of the patch and not the v2
> > version Andy sent.
> 
> That's one of the problems with having an out-of-tree tree.  Please
> don't do that at all anymore.

I don't see a difference in a in-tree tree and and out-of-tree tree when 
applying patches somewhere else out of order. In both situations we have a 
merge conflict (not that the scm says "omg, i cannot merge it" but that the 
thing doesn't compile after the merge). Not that it would care much that there 
is a merge conflict - it only has to be resolved one way or the other. And I 
don't force the submitter to do it (I couldn't do it anyway), but try to help 
to resolve it for the actual maintainer.

I always thought that even when the source is in the kernel (or in staging) 
that there are still a maintainer responsible for it. That this person has to 
go through the patches and look if they do whatever they claim to do and that 
this isn't against what the original implementation had to do or should do.

In case of batman-adv those maintainers are called by the names Marek Lindner 
and Simon Wunderlich. None of those names sound like GregKH or random guy (aka 
me). I know that you are the staging maintainer and you have more to say, but 
wouldn't it be more healthy that the guys who know more details about it can 
take a look at the non critical stuff?

And the only reason I proposed (not forced, only recommended) that will be 
send later for inclusion in 2.6.38 was that you told us that the day a new 
kernel (2.6.36) gets released is too late to get new stuff (other than fixes) 
in your tree for 2.6.37.

> I'll go apply this patch to mine, and you can handle any merge issues if
> you continue to wish to keep an external tree (hint, I STRONGLY
> recommend that you do not, for these reasons and many others.)

The development of batman-adv is mainly done by people which need it 
externally - so out of kernel. That means if we are not allowed anymore to 
have some kind of external tree that we can use, it must be done the other way 
around aka compat-wireless like and without the ability to test experimental 
stuff with the community unless by sending it to you and reverting it before a 
new linux release is made. Otherwise we would only have a external tree which 
is in another form (quilt, loose patches, ...) and would be nothing different 
than what we have currently.

The current way is not to say that we come first, but to keep things 
organized. Take for example Andy's patch - it was in my own opinion to late 
for 2.6.37 and it was nothing real critical (but nevertheless quite helpful). 
So queuing it up with the stuff for 2.6.38 seemed to be a good idea. In that 
time also the affected other code parts could be fixed by the actual author 
and everything flows back to you as a complete package.

And in my own opinion the external tree was quite helpful to see incorrect 
merge resolutions...

Everything above is my own opinion and does not strictly reflect the positions 
and policies of the batman-adv project or the actual maintainers.

thanks,
	Sven

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

Re: [B.A.T.M.A.N.] [resend][PATCHv2] staging: batman-adv: remove useless addr_to_string()

[Greg KH]

On Thu, Oct 21, 2010 at 02:41:37PM +0200, Sven Eckelmann wrote:
> Greg KH wrote:
> > On Thu, Oct 21, 2010 at 12:16:37AM +0200, Sven Eckelmann wrote:
> > > On Wed, Oct 20, 2010 at 01:57:49PM -0700, Greg KH wrote:
> > > > On Wed, Oct 20, 2010 at 10:51:15PM +0200, Sven Eckelmann wrote:
> > > > > On Wed, Oct 20, 2010 at 06:47:44PM +0300, Andy Shevchenko wrote:
> > > > > > Since all *printf() methods in the kernel understand '%pM' modifier
> > > > > > the conversion to the string is useless beforehand.
> > > > > > 
> > > > > > Additionally this patch decreases batman_if structure by 20 bytes.
> > > > > 
> > > > > Thanks for your patch. I have problems with compiling due to other
> > > > > patches in the queue. I will fix that and recommend it as patch for
> > > > > 2.6.38.
> > > > 
> > > > What do you mean by this?  It applies just fine to my tree, so why
> > > > can't I take it now?
> > > 
> > > If you want then do so, but the stuff in batman-adv's master must be
> > > fixed so they have to apply the v3 version of the patch and not the v2
> > > version Andy sent.
> > 
> > That's one of the problems with having an out-of-tree tree.  Please
> > don't do that at all anymore.
> 
> I don't see a difference in a in-tree tree and and out-of-tree tree when 
> applying patches somewhere else out of order. In both situations we have a 
> merge conflict (not that the scm says "omg, i cannot merge it" but that the 
> thing doesn't compile after the merge).

Not true at all, the in-linux-next tree builds just fine with this
patch.  In fact, it's now in linux-next already.

> I always thought that even when the source is in the kernel (or in staging) 
> that there are still a maintainer responsible for it. That this person has to 
> go through the patches and look if they do whatever they claim to do and that 
> this isn't against what the original implementation had to do or should do.

Yes, but sometimes, especially for trivial patches, the maintainer is
routed around and patches go in through other trees.

Remember a maintainer is not someone who can say "no" to all patches
that comes in, sorry, we don't work that way.

> > I'll go apply this patch to mine, and you can handle any merge issues if
> > you continue to wish to keep an external tree (hint, I STRONGLY
> > recommend that you do not, for these reasons and many others.)
> 
> The development of batman-adv is mainly done by people which need it 
> externally - so out of kernel. That means if we are not allowed anymore to 
> have some kind of external tree that we can use, it must be done the other way 
> around aka compat-wireless like and without the ability to test experimental 
> stuff with the community unless by sending it to you and reverting it before a 
> new linux release is made. Otherwise we would only have a external tree which 
> is in another form (quilt, loose patches, ...) and would be nothing different 
> than what we have currently.

As your stuff is now in the kernel, I would recommend dropping your
external tree.  It will only cause more problems in the future.

thanks,

greg k-h

Re: [B.A.T.M.A.N.] [resend][PATCHv2] staging: batman-adv: remove useless addr_to_string()

[Sven Eckelmann]

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

Greg KH wrote:
> On Thu, Oct 21, 2010 at 02:41:37PM +0200, Sven Eckelmann wrote:
> > Greg KH wrote:
> > > On Thu, Oct 21, 2010 at 12:16:37AM +0200, Sven Eckelmann wrote:
> > > > On Wed, Oct 20, 2010 at 01:57:49PM -0700, Greg KH wrote:
> > > > > On Wed, Oct 20, 2010 at 10:51:15PM +0200, Sven Eckelmann wrote:
> > > > > > On Wed, Oct 20, 2010 at 06:47:44PM +0300, Andy Shevchenko wrote:
> > > > > > > Since all *printf() methods in the kernel understand '%pM'
> > > > > > > modifier the conversion to the string is useless beforehand.
> > > > > > > 
> > > > > > > Additionally this patch decreases batman_if structure by 20
> > > > > > > bytes.
> > > > > > 
> > > > > > Thanks for your patch. I have problems with compiling due to
> > > > > > other patches in the queue. I will fix that and recommend it as
> > > > > > patch for 2.6.38.
> > > > > 
> > > > > What do you mean by this?  It applies just fine to my tree, so why
> > > > > can't I take it now?
> > > > 
> > > > If you want then do so, but the stuff in batman-adv's master must be
> > > > fixed so they have to apply the v3 version of the patch and not the
> > > > v2 version Andy sent.
> > > 
> > > That's one of the problems with having an out-of-tree tree.  Please
> > > don't do that at all anymore.
> > 
> > I don't see a difference in a in-tree tree and and out-of-tree tree when
> > applying patches somewhere else out of order. In both situations we have
> > a merge conflict (not that the scm says "omg, i cannot merge it" but
> > that the thing doesn't compile after the merge).
> 
> Not true at all, the in-linux-next tree builds just fine with this
> patch.  In fact, it's now in linux-next already.

He? I never said that it breaks stuff in your staging tree.

> > I always thought that even when the source is in the kernel (or in
> > staging) that there are still a maintainer responsible for it. That this
> > person has to go through the patches and look if they do whatever they
> > claim to do and that this isn't against what the original implementation
> > had to do or should do.
> 
> Yes, but sometimes, especially for trivial patches, the maintainer is
> routed around and patches go in through other trees.
> 
> Remember a maintainer is not someone who can say "no" to all patches
> that comes in, sorry, we don't work that way.

What? I no batman-adv maintainer said no to patches on the lkml or other linux 
related mailing lists as far as I can remember, but postponed them or 
recommended changes.

There were patches dropped in the past which didn't make sense or created more 
problems than they solved - but that was even before batman-adv entered 
staging.

thanks,
	Sven

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