[ofa-general] Re: [PATCH] opensm/osm_ucast_lash.c: Handle malloc failures better

[Sasha Khapyorsky]

Hi Hal,

On 16:22 Mon 27 Jul     , Hal Rosenstock wrote:
> 
> Signed-off-by: Hal Rosenstock <hal.rosenstock at gmail.com>
> ---
> diff --git a/opensm/opensm/osm_ucast_lash.c b/opensm/opensm/osm_ucast_lash.c
> index 7133e25..a139bdb 100644
> --- a/opensm/opensm/osm_ucast_lash.c
> +++ b/opensm/opensm/osm_ucast_lash.c
> @@ -296,8 +296,8 @@ static void shortest_path(lash_t * p_lash, int ir)
>  	cl_list_destroy(&bfsq);
>  }
>  
> -static void generate_routing_func_for_mst(lash_t * p_lash, int sw_id,
> -					  reachable_dest_t ** destinations)
> +static boolean_t generate_routing_func_for_mst(lash_t * p_lash, int sw_id,
> +					       reachable_dest_t ** destinations)

I think that 'int' is more suitable and simpler for using as return
status value.

>  {
>  	int i, next_switch;
>  	switch_t *sw = p_lash->switches[sw_id];
> @@ -306,7 +306,8 @@ static void generate_routing_func_for_mst(lash_t * p_lash, int sw_id,
>  
>  	for (i = 0; i < num_channels; i++) {
>  		next_switch = sw->dij_channels[i];
> -		generate_routing_func_for_mst(p_lash, next_switch, &dest);
> +		if (!generate_routing_func_for_mst(p_lash, next_switch, &dest))
> +			return FALSE;

BTW, it looks like a BFS. Could recursion be avoided here?

>  
>  		i_dest = dest;
>  		prev = i_dest;
> @@ -327,9 +328,15 @@ static void generate_routing_func_for_mst(lash_t * p_lash, int sw_id,
>  	}
>  
>  	i_dest = (reachable_dest_t *) malloc(sizeof(reachable_dest_t));
> -	i_dest->switch_id = sw->id;
> -	i_dest->next = concat_dest;
> +	if (i_dest) {
> +		i_dest->switch_id = sw->id;
> +		i_dest->next = concat_dest;
> +	}
>  	*destinations = i_dest;
> +	if (i_dest)
> +		return TRUE;
> +	else
> +		return FALSE;
>  }

And then:

	i_dest = malloc(sizeof(reachable_dest_t));
	if (!i_dest)
		return -1;

, that's all.

Sasha