Re: [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Remove old fragmentation code

[Marek Lindner <lindner_marek@yahoo.de>]

On Sunday, April 21, 2013 00:17:32 Martin Hundebøll wrote:
> @@ -592,9 +591,9 @@ static bool batadv_dat_send_data(struct batadv_priv
> *bat_priv, goto free_orig;
> 
>  		tmp_skb = pskb_copy(skb, GFP_ATOMIC);
> -		if (!batadv_unicast_4addr_prepare_skb(bat_priv, tmp_skb,
> -						      cand[i].orig_node,
> -						      packet_subtype)) {
> +		if (!batadv_send_skb_4addr_prepare_unicast(bat_priv, tmp_skb,
> +							   cand[i].orig_node,
> +							   packet_subtype)) {

I think you mangled the "unicast_4addr" name a little. Afaik "4addr" was meant 
to follow "unicast" to diffirentiate between "unicast" and "unicast_4addr". 
However, I'd have no problem if you simply dropped the "unicast" part. For 
example: batadv_send_skb_4addr_prepare()


> @@ -977,10 +976,10 @@ bool batadv_dat_snoop_incoming_arp_request(struct
> batadv_priv *bat_priv, * that a node not using the 4addr packet format
> doesn't support it. */
>  	if (hdr_size == sizeof(struct batadv_unicast_4addr_packet))
> -		err = batadv_unicast_4addr_send_skb(bat_priv, skb_new,
> +		err = batadv_send_skb_4addr_unicast(bat_priv, skb_new,
>  						    BATADV_P_DAT_CACHE_REPLY);

If you decide to skip the "unicast" part above feel free to drop it here too.


> +/**
> + * batadv_send_generic_unicast_skb - send an skb as unicast
> + * @bat_priv: the bat priv with all the soft interface information
> + * @skb: payload to send
> + * @packet_type: the batman unicast packet type to use
> + * @packet_subtype: the batman packet subtype. It is ignored if packet_type 
> + *		    not BATADV_UNICAT_4ADDR
> + *
> + * Returns 1 in case of error or 0 otherwise
> + */

Multi-line kernel doc explanations should be indented by one space. 
If the function returns 0 or 1 only it should be made boolean or the return 
value should use a readable define.

Cheers,
Marek