[ofa-general] Re: [PATCH] opensm/osm_mcast_tbl.c: osm_mcast_tbl_get_block returns boolean

[Hal Rosenstock]

On Wed, Aug 12, 2009 at 5:03 AM, Sasha Khapyorsky <sashak at voltaire.com>wrote:

> On 09:43 Fri 07 Aug     , Hal Rosenstock wrote:
> >
> > so use TRUE/FALSE rather than IB_INVALID_PARMETER
> >
> > Signed-off-by: Hal Rosenstock <hal.rosenstock at gmail.com>
> > ---
> > diff --git a/opensm/opensm/osm_mcast_tbl.c
> b/opensm/opensm/osm_mcast_tbl.c
> > index 82850be..38c06c1 100644
> > --- a/opensm/opensm/osm_mcast_tbl.c
> > +++ b/opensm/opensm/osm_mcast_tbl.c
> > @@ -273,7 +273,7 @@ osm_mcast_tbl_get_block(IN osm_mcast_tbl_t * const
> p_tbl,
> >       mlid_start_ho = (uint16_t) (block_num * IB_MCAST_BLOCK_SIZE);
> >
> >       if (mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1 > p_tbl->max_mlid_ho)
> > -             return (IB_INVALID_PARAMETER);
> > +             return (TRUE);
>
> In this case p_block array is not initialized, so just returning 'TRUE'
> is not a good idea.


That's how it's handled now in the code.


>
>
> Actually if we are hitting this case it can indicate an inconsistent
> mcast_tbl initialization


That's one possibility. The other would be a SA query for a specific
block that is not validated first.


> - I would suggest to rework this part and likely to drop this check at all.


Dropping it should be OK as long as all callers validate prior to calling
(and it looks like they do).

-- 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/20090812/5a52d5e1/attachment.html>