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

[Hal Rosenstock]

Hi Sasha,

On Thu, Jul 30, 2009 at 5:33 AM, Sasha Khapyorsky <sashak at voltaire.com>wrote:

> 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?


Why do you want to eliminate the recursion ?


>
>
> >
> >               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.


I'm not following what you mean here.

-- Hal


>
> Sasha
>  _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit
> http://openib.org/mailman/listinfo/openib-general
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/general/attachments/20090730/144eb672/attachment.html>