Re: [B.A.T.M.A.N.] [PATCHv5 4/9] batman-adv: Distributed ARP Table - add ARP parsing functions

[Antonio Quartulli <ordex@autistici.org>]

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

On Sat, Feb 11, 2012 at 10:09:30PM +0800, Marek Lindner wrote:
> On Friday, February 10, 2012 07:41:37 Antonio Quartulli wrote:
> > +static inline void bat_dbg_arp(struct bat_priv *bat_priv,
> > +                              struct sk_buff *skb, uint16_t type) {
> > +       char buf[30];
> > +       const char *type_str[] = { "REQUEST", "REPLY", "RREQUEST",
> > "RREPLY", +                                     "InREQUEST", "InREPLY",
> > "NAK" }; +
> > +       if (type >= 1 && type <= ARRAY_SIZE(type_str))
> > +               scnprintf(buf, sizeof(buf), "%s", type_str[type - 1]);
> > +       else
> > +               scnprintf(buf, sizeof(buf), "UNKNOWN (%hu)", type);
> > +
> > +       bat_dbg(DBG_ARP, bat_priv, "ARP message of type %s recognised "
> > +               "[src: %pM-%pI4 dst: %pM-%pI4]\n", buf, ARP_HW_SRC(skb),
> > +               &ARP_IP_SRC(skb), ARP_HW_DST(skb), &ARP_IP_DST(skb));
> > +}
> 
> Every time this fucntion is called an additional bat_dgb() call is made 
> directly before that (as far as I can tell). Wouldn't it make sense to include 
> this extra message as parameter to bat_dbg_arp() ?

I'd say it's a good idea. Despite of the current usage, I thought this function
to be responsible to print the content of the message only. The fact that a
bat_dbg() invocation always precedes its is just a coincident.

But adding a another argument (which can eventually be NULL) makes much sense :)
Thanks!

Cheers,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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