Re: [B.A.T.M.A.N.] [PATCH v2 3/3] batman-adv: throughput meter implementation

[Antonio Quartulli <antonio.quartulli@open-mesh.com>]

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

Sven,

a comment is inline.

On Fri, Apr 01, 2016 at 12:54:36PM +0200, Sven Eckelmann wrote:

[...]

> +/**
> + * batadv_tp_send - main sending thread of a tp meter session
> + * @arg: address of the related tp_vars
> + *
> + * Return: nothing, this function never returns
> + */
> +static int batadv_tp_send(void *arg)
> +{
> +	struct batadv_tp_vars *tp_vars = arg;
> +	struct batadv_priv *bat_priv = tp_vars->bat_priv;
> +	struct batadv_hard_iface *primary_if = NULL;
> +	struct batadv_orig_node *orig_node = NULL;
> +	size_t payload_len, packet_len;
> +	int err = 0;
> +
> +	if (unlikely(tp_vars->role != BATADV_TP_SENDER)) {
> +		err = BATADV_TP_DST_UNREACHABLE;
> +		goto out;
> +	}
> +
> +	orig_node = batadv_orig_hash_find(bat_priv, tp_vars->other_end);
> +	if (unlikely(!orig_node)) {
> +		err = BATADV_TP_DST_UNREACHABLE;
> +		goto out;
> +	}
> +
> +	primary_if = batadv_primary_if_get_selected(bat_priv);
> +	if (unlikely(!primary_if)) {
> +		err = BATADV_TP_DST_UNREACHABLE;
> +		goto out;
> +	}

in case of error we modify err and then we jump to out, but:

[...]

> +out:
> +	if (likely(primary_if))
> +		batadv_hardif_put(primary_if);
> +	if (likely(orig_node))
> +		batadv_orig_node_put(orig_node);
> +
> +	batadv_tp_sender_end(bat_priv, tp_vars);
> +	batadv_tp_sender_cleanup(bat_priv, tp_vars);
> +
> +	batadv_tp_vars_put(tp_vars);
> +
> +	do_exit(0);
> +}

shouldn't we propagate the value saved in err to tp_vars->reason ?

We found this error while using the tp_meter, because we tried to run it
against a non-existing originator and batctl crashed.


Cheers,

-- 
Antonio Quartulli

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