Re: [B.A.T.M.A.N.] [PATCHv2 1/7] batman-adv: Return reason for failure in batadv_check_unicast_packet()

[Antonio Quartulli <ordex@autistici.org>]

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

On Sat, Dec 15, 2012 at 04:10:17PM +0800, Marek Lindner wrote:
> On Friday, December 14, 2012 14:32:46 Martin Hundebøll wrote:
> >  static int batadv_check_unicast_packet(struct sk_buff *skb, int hdr_size)
> >  {
> >  	struct ethhdr *ethhdr;
> > @@ -564,15 +574,15 @@ static int batadv_check_unicast_packet(struct sk_buff
> > *skb, int hdr_size)
> > 
> >  	/* packet with unicast indication but broadcast recipient */
> >  	if (is_broadcast_ether_addr(ethhdr->h_dest))
> > -		return -1;
> > +		return -2;
> > 
> >  	/* packet with broadcast sender address */
> >  	if (is_broadcast_ether_addr(ethhdr->h_source))
> > -		return -1;
> > +		return -3;
> > 
> >  	/* not for me */
> >  	if (!batadv_is_my_mac(ethhdr->h_dest))
> > -		return -1;
> > +		return -4;
> > 
> >  	return 0;
> >  }
> 
> Returning meaningless numbers isn't the way to go. Either we make our own 
> defines or we re-use our E***** friends.

If I am not mistaken you only use "-4" in your network coding code. Maybe you
want to reduce this distinction to two cases only?

e.g.
* -EINVAL : incorrect header or bad fields (-1, -2, -3)..the packet is not valid
  in the end
* something else for your case..what about -EREMOTE ?


> 
> By the way, I suggest to send this patch separately. It increases the chance 
> of getting merged faster and reduces your patchset.

I agree on this. It would simplify your daily rebasing task. And when/if you do
so, remember to change the commit message: networking does not exist yet.

Cheers,


-- 
Antonio Quartulli

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

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