diff mbox

[RFC] batman-adv: pass value in batadv_hash_bytes

Message ID 1352886828-24651-1-git-send-email-siwu@hrz.tu-chemnitz.de (mailing list archive)
State Accepted, archived
Commit 803bae00f0ed480ed4be0b28792847162b0b91bc
Headers

Commit Message

Simon Wunderlich Nov. 14, 2012, 9:53 a.m. UTC 
  Passing the hash value by reference creates unneeded overhead. Pass by
value instead.

Reported-by: David Miller <davem@davemloft.net>
Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
---
 bridge_loop_avoidance.c |    8 ++++----
 hash.h                  |   12 ++++++++----
 2 files changed, 12 insertions(+), 8 deletions(-)

Comments

Antonio Quartulli Nov. 14, 2012, 11:59 a.m. UTC | #1 
On Wed, Nov 14, 2012 at 10:53:48 +0100, Simon Wunderlich wrote:
> Passing the hash value by reference creates unneeded overhead. Pass by
> value instead.
> 
> Reported-by: David Miller <davem@davemloft.net>
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>

I think this was not meant to be an RFC :-)
And I suppose this patch should be applied to next and no to master, because it
should be merged with the next pull request.

Cheers,
Marek Lindner Nov. 14, 2012, 1:13 p.m. UTC | #2 
On Wednesday, November 14, 2012 19:59:07 Antonio Quartulli wrote:
>   On Wed, Nov 14, 2012 at 10:53:48 +0100, Simon Wunderlich wrote:
> > Passing the hash value by reference creates unneeded overhead. Pass by
> > value instead.
> >
> > 
> >
> > Reported-by: David Miller <davem@davemloft.net>
> > Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
> 
> I think this was not meant to be an RFC :-)
> And I suppose this patch should be applied to next and no to master,
> because it should be merged with the next pull request.

Aren't we supposed to remove the inline tag as well ?

Cheers,
Marek
Sven Eckelmann Nov. 14, 2012, 1:21 p.m. UTC | #3 
On Wednesday 14 November 2012 21:13:36 Marek Lindner wrote:
> On Wednesday, November 14, 2012 19:59:07 Antonio Quartulli wrote:
> >   On Wed, Nov 14, 2012 at 10:53:48 +0100, Simon Wunderlich wrote:
> > > Passing the hash value by reference creates unneeded overhead. Pass by
> > > value instead.
> > > 
> > > 
> > > 
> > > Reported-by: David Miller <davem@davemloft.net>
> > > Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
> > 
> > I think this was not meant to be an RFC :-)
> > And I suppose this patch should be applied to next and no to master,
> > because it should be merged with the next pull request.
> 
> Aren't we supposed to remove the inline tag as well ?

Not in the header -- only in the c file

Kind regards,
	Sven
Marek Lindner Nov. 14, 2012, 4:37 p.m. UTC | #4 
On Wednesday, November 14, 2012 17:53:48 Simon Wunderlich wrote:
> Passing the hash value by reference creates unneeded overhead. Pass by
> value instead.
> 
> Reported-by: David Miller <davem@davemloft.net>
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
> ---
>  bridge_loop_avoidance.c |    8 ++++----
>  hash.h                  |   12 ++++++++----
>  2 files changed, 12 insertions(+), 8 deletions(-)

Applied in revision 803bae0.

Thanks,
Marek
diff mbox

Patch

diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c
index b6da6ae..fc56ea2 100644
--- a/bridge_loop_avoidance.c
+++ b/bridge_loop_avoidance.c
@@ -43,8 +43,8 @@  static inline uint32_t batadv_choose_claim(const void *data, uint32_t size)
 	struct batadv_claim *claim = (struct batadv_claim *)data;
 	uint32_t hash = 0;
 
-	batadv_hash_bytes(&hash, &claim->addr, sizeof(claim->addr));
-	batadv_hash_bytes(&hash, &claim->vid, sizeof(claim->vid));
+	hash = batadv_hash_bytes(hash, &claim->addr, sizeof(claim->addr));
+	hash = batadv_hash_bytes(hash, &claim->vid, sizeof(claim->vid));
 
 	hash += (hash << 3);
 	hash ^= (hash >> 11);
@@ -60,8 +60,8 @@  static inline uint32_t batadv_choose_backbone_gw(const void *data,
 	struct batadv_claim *claim = (struct batadv_claim *)data;
 	uint32_t hash = 0;
 
-	batadv_hash_bytes(&hash, &claim->addr, sizeof(claim->addr));
-	batadv_hash_bytes(&hash, &claim->vid, sizeof(claim->vid));
+	hash = batadv_hash_bytes(hash, &claim->addr, sizeof(claim->addr));
+	hash = batadv_hash_bytes(hash, &claim->vid, sizeof(claim->vid));
 
 	hash += (hash << 3);
 	hash ^= (hash >> 11);
diff --git a/hash.h b/hash.h
index f173427..e053339 100644
--- a/hash.h
+++ b/hash.h
@@ -86,17 +86,21 @@  static inline void batadv_hash_delete(struct batadv_hashtable *hash,
  *	@hash: previous hash value
  *	@data: data to be hashed
  *	@size: number of bytes to be hashed
+ *
+ *	Returns the new hash value.
  */
-static inline void batadv_hash_bytes(uint32_t *hash, void *data, uint32_t size)
+static inline uint32_t batadv_hash_bytes(uint32_t hash, void *data,
+					 uint32_t size)
 {
 	const unsigned char *key = data;
 	int i;
 
 	for (i = 0; i < size; i++) {
-		*hash += key[i];
-		*hash += (*hash << 10);
-		*hash ^= (*hash >> 6);
+		hash += key[i];
+		hash += (hash << 10);
+		hash ^= (hash >> 6);
 	}
+	return hash;
 }
 
 /**