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

[Sasha Khapyorsky]

On 05:49 Thu 30 Jul     , Hal Rosenstock wrote:
> > 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 ?

Non-recursive looping doesn't require stack allocations and normally
faster (and simpler), it is almost always better to not use recursion
when possible.

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

Those three lines do the same job as this chunk of the patch, but
shorter.

Sasha