Updating orig_str for debug

Message ID 20091203002407.GA10048@Linus-Debian (mailing list archive)
State Accepted, archived
Headers

Commit Message

Linus Lüssing Dec. 3, 2009, 12:24 a.m. UTC
  In commit 1488 the orig_str has been removed from
purge_orig_neighbours(). Apparently, this breaks compilation when
enabling debug-flags, as bat_dbg() is expecting this variable as a
parameter. Actually, this variable has never been initialised also
before commit 1488 (which was probably the reason for mistakenly
removing it).
This tiny patch adds the orig_str again, but initialises it now as
well.

Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
  

Comments

Marek Lindner Dec. 3, 2009, 2:45 a.m. UTC | #1
On Thursday 03 December 2009 08:24:07 Linus Lüssing wrote:
> In commit 1488 the orig_str has been removed from
> purge_orig_neighbours(). Apparently, this breaks compilation when
> enabling debug-flags, as bat_dbg() is expecting this variable as a
> parameter. Actually, this variable has never been initialised also
> before commit 1488 (which was probably the reason for mistakenly
> removing it).
> This tiny patch adds the orig_str again, but initialises it now as
> well.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@web.de>

Thanks for your patch - I just applied it.
By the way, it was a bit troublesome - somehow patch did not like your email. 
I got malformed patch error messages. Not sure what the issue is.

Regards,
Marek
  
Gus Wirth Dec. 3, 2009, 2:46 a.m. UTC | #2
On 12/02/2009 04:24 PM, Linus Lüssing wrote:
> In commit 1488 the orig_str has been removed from
> purge_orig_neighbours(). Apparently, this breaks compilation when
> enabling debug-flags, as bat_dbg() is expecting this variable as a
> parameter. Actually, this variable has never been initialised also
> before commit 1488 (which was probably the reason for mistakenly
> removing it).
> This tiny patch adds the orig_str again, but initialises it now as
> well.
>
> Signed-off-by: Linus Lüssing<linus.luessing@web.de>
> ---
>
> Index: originator.c
> ===================================================================
> --- originator.c        (revision 1489)
> +++ originator.c        (working copy)
> @@ -163,7 +163,7 @@
>                                   struct neigh_node **best_neigh_node)
>   {
>          struct list_head *list_pos, *list_pos_tmp;
> -       char neigh_str[ETH_STR_LEN];
> +       char neigh_str[ETH_STR_LEN], orig_str[ETH_STR_LEN];
>          struct neigh_node *neigh_node;
>          bool neigh_purged = false;
>
> @@ -179,6 +179,7 @@
>                                  ((PURGE_TIMEOUT * HZ) / 1000)))) {
>
>                          addr_to_string(neigh_str, neigh_node->addr);
> +                       addr_to_string(orig_str, orig_node->orig);
>                          bat_dbg(DBG_BATMAN, "Neighbour timeout: originator %s, neighbour: %s, last_valid %lu\n", orig_str, neigh_str, (neigh_node->last_valid / HZ));
>
>                          neigh_purged = true;
>

=================

Shouldn't the "addr_to_string(orig_str, orig_node->orig);" be moved out 
side the loop to near the top of the function? There is only one 
originator node and there is no reason to iterate over it. One time 
should suffice.

Gus
  
Marek Lindner Dec. 3, 2009, 2:48 a.m. UTC | #3
Hi Gus,

> Shouldn't the "addr_to_string(orig_str, orig_node->orig);" be moved out
> side the loop to near the top of the function? There is only one
> originator node and there is no reason to iterate over it. One time
> should suffice.

you are probably right but I know that Andrew has a patch in his pipeline 
which will remove these addr_to_string() calls entirely (printk can handle mac 
addresses for us). Until then we can live with this workaround. :)

Regards,
Marek
  
Andrew Lunn Dec. 3, 2009, 5:53 a.m. UTC | #4
> you are probably right but I know that Andrew has a patch in his
> pipeline which will remove these addr_to_string() calls entirely
> (printk can handle mac addresses for us). Until then we can live
> with this workaround. :)

I've been holding off on this patch because of Simon's big lock
removal patch. The two will probably clash.

However the deadlocks Linus just posted suggests Simon's patch is
still in the experimental stage. My patch should be low risk, so maybe
it can go in first. I will submit a patch ASAP.

   Andrew
  

Patch

Index: originator.c
===================================================================
--- originator.c        (revision 1489)
+++ originator.c        (working copy)
@@ -163,7 +163,7 @@ 
                                 struct neigh_node **best_neigh_node)
 {
        struct list_head *list_pos, *list_pos_tmp;
-       char neigh_str[ETH_STR_LEN];
+       char neigh_str[ETH_STR_LEN], orig_str[ETH_STR_LEN];
        struct neigh_node *neigh_node;
        bool neigh_purged = false;
 
@@ -179,6 +179,7 @@ 
                                ((PURGE_TIMEOUT * HZ) / 1000)))) {
 
                        addr_to_string(neigh_str, neigh_node->addr);
+                       addr_to_string(orig_str, orig_node->orig);
                        bat_dbg(DBG_BATMAN, "Neighbour timeout: originator %s, neighbour: %s, last_valid %lu\n", orig_str, neigh_str, (neigh_node->last_valid / HZ)); 
 
                        neigh_purged = true;