[PATCH 1/2] batman-adv: count_real_packets() in batman-adv assumes char is signed

[PATCH 1/2] batman-adv: count_real_packets() in batman-adv assumes char is signed

[David Howells]

count_real_packets() in batman-adv assumes char is signed, and returns -1
through it:

net/batman-adv/routing.c: In function 'receive_bat_packet':
net/batman-adv/routing.c:739: warning: comparison is always false due to limited range of data type

Use int instead.

This is also looks a bit weird as (presumably signed) is_duplicate is
constructed by OR'ding together the unsigned results of get_bit_status()
(though the latter only returns 0 or 1).

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marek Lindner <lindner_marek@yahoo.de>
cc: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
cc: Sven Eckelmann <sven@narfation.org>
cc: b.a.t.m.a.n@lists.open-mesh.org
cc: netdev@vger.kernel.org
---

 net/batman-adv/routing.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index bb1c3ec..3075fcb 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -531,15 +531,15 @@ static int window_protected(struct bat_priv *bat_priv,
  *  -1 the packet is old and has been received while the seqno window
  *     was protected. Caller should drop it.
  */
-static char count_real_packets(struct ethhdr *ethhdr,
-			       struct batman_packet *batman_packet,
-			       struct hard_iface *if_incoming)
+static int count_real_packets(struct ethhdr *ethhdr,
+			      struct batman_packet *batman_packet,
+			      struct hard_iface *if_incoming)
 {
 	struct bat_priv *bat_priv = netdev_priv(if_incoming->soft_iface);
 	struct orig_node *orig_node;
 	struct neigh_node *tmp_neigh_node;
 	struct hlist_node *node;
-	char is_duplicate = 0;
+	uint8_t is_duplicate = 0;
 	int32_t seq_diff;
 	int need_update = 0;
 	int set_mark, ret = -1;
@@ -608,7 +608,7 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 	char has_directlink_flag;
 	char is_my_addr = 0, is_my_orig = 0, is_my_oldorig = 0;
 	char is_broadcast = 0, is_bidirectional, is_single_hop_neigh;
-	char is_duplicate;
+	int is_duplicate;
 	uint32_t if_incoming_seqno;
 
 	/* Silently drop when the batman packet is actually not a

[PATCH 2/2] batman-adv: compare_eth() should take const pointer arguments

[David Howells]

compare_eth() should take const pointer arguments so that it can be passed
const pointers without the need for a cast, leading to:

net/batman-adv/vis.c: In function 'vis_data_insert_interface':
net/batman-adv/vis.c:146: warning: passing argument 2 of 'compare_eth' discards qualifiers from pointer target type

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marek Lindner <lindner_marek@yahoo.de>
cc: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
cc: Sven Eckelmann <sven@narfation.org>
cc: b.a.t.m.a.n@lists.open-mesh.org
cc: netdev@vger.kernel.org
---

 net/batman-adv/main.h |    2 +-
 net/batman-adv/vis.c  |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
index 148b49e..e6fc798 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -172,7 +172,7 @@ static inline void bat_dbg(char type __always_unused,
  *
  * note: can't use compare_ether_addr() as it requires aligned memory
  */
-static inline int compare_eth(void *data1, void *data2)
+static inline int compare_eth(const void *data1, const void *data2)
 {
 	return (memcmp(data1, data2, ETH_ALEN) == 0 ? 1 : 0);
 }
diff --git a/net/batman-adv/vis.c b/net/batman-adv/vis.c
index c39f20c..34053ac 100644
--- a/net/batman-adv/vis.c
+++ b/net/batman-adv/vis.c
@@ -143,7 +143,7 @@ static void vis_data_insert_interface(const uint8_t *interface,
 	struct hlist_node *pos;
 
 	hlist_for_each_entry(entry, pos, if_list, list) {
-		if (compare_eth(entry->addr, (void *)interface))
+		if (compare_eth(entry->addr, interface))
 			return;
 	}
 

Re: [PATCH 2/2] batman-adv: compare_eth() should take const pointer arguments

[Sven Eckelmann]

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

David Howells wrote:
> compare_eth() should take const pointer arguments so that it can be passed
> const pointers without the need for a cast, leading to:
> 
> net/batman-adv/vis.c: In function 'vis_data_insert_interface':
> net/batman-adv/vis.c:146: warning: passing argument 2 of 'compare_eth'
> discards qualifiers from pointer target type
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Marek Lindner <lindner_marek@yahoo.de>
> cc: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
> cc: Sven Eckelmann <sven@narfation.org>
> cc: b.a.t.m.a.n@lists.open-mesh.org
> cc: netdev@vger.kernel.org
> ---
> 
>  net/batman-adv/main.h |    2 +-
>  net/batman-adv/vis.c  |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
> index 148b49e..e6fc798 100644
> --- a/net/batman-adv/main.h
> +++ b/net/batman-adv/main.h
> @@ -172,7 +172,7 @@ static inline void bat_dbg(char type __always_unused,
>   *
>   * note: can't use compare_ether_addr() as it requires aligned memory
>   */
> -static inline int compare_eth(void *data1, void *data2)
> +static inline int compare_eth(const void *data1, const void *data2)
>  {
>  	return (memcmp(data1, data2, ETH_ALEN) == 0 ? 1 : 0);
>  }
> diff --git a/net/batman-adv/vis.c b/net/batman-adv/vis.c
> index c39f20c..34053ac 100644
> --- a/net/batman-adv/vis.c
> +++ b/net/batman-adv/vis.c
> @@ -143,7 +143,7 @@ static void vis_data_insert_interface(const uint8_t
> *interface, struct hlist_node *pos;
> 
>  	hlist_for_each_entry(entry, pos, if_list, list) {
> -		if (compare_eth(entry->addr, (void *)interface))
> +		if (compare_eth(entry->addr, interface))
>  			return;
>  	}

Sry, but this patch doesn't apply here (net-next-2.6/linux-next)

Kind regards,
	Sven

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

Re: [PATCH 1/2] batman-adv: count_real_packets() in batman-adv assumes char is signed

[Sven Eckelmann]

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

David Howells wrote:
> count_real_packets() in batman-adv assumes char is signed, and returns -1
> through it:
> 
> net/batman-adv/routing.c: In function 'receive_bat_packet':
> net/batman-adv/routing.c:739: warning: comparison is always false due to
> limited range of data type
> 
> Use int instead.
> 

[...]
> -static char count_real_packets(struct ethhdr *ethhdr,
> -			       struct batman_packet *batman_packet,
> -			       struct hard_iface *if_incoming)
> +static int count_real_packets(struct ethhdr *ethhdr,
> +			      struct batman_packet *batman_packet,
> +			      struct hard_iface *if_incoming)
>  {


This one doesn't apply on linux-next/net-next-2.6, but I will fix it by hand.

Thanks,
	Sven

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

Re: [PATCH 1/2] batman-adv: count_real_packets() in batman-adv assumes char is signed

[Sven Eckelmann]

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

On Wednesday 15 June 2011 01:51:32 David Howells wrote:
> count_real_packets() in batman-adv assumes char is signed, and returns -1
> through it:
> 
> net/batman-adv/routing.c: In function 'receive_bat_packet':
> net/batman-adv/routing.c:739: warning: comparison is always false due to
> limited range of data type
> 
> Use int instead.
> 
> This is also looks a bit weird as (presumably signed) is_duplicate is
> constructed by OR'ding together the unsigned results of get_bit_status()
> (though the latter only returns 0 or 1).

Sry, had to catch the train and had no time to explain it further.

It is correct that is_duplicate will only have 0 and 1 stored, but the 
window_protected function (called before the loop) may detect that the packet 
has to be dropped and we return in that case -1.

I don't know who started to use char in those places, but thanks for reminding 
me how much I hate it and that I wanted to check the rest of the code. :)

I will submit the corrected patch in a pull request later this week to David 
S. Miller.

Thanks,
	Sven

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