[ofa-general] Re: PATCH 1/1 - fix kernel crash in mad.c when IB_MAD_RESULT_(SUCCESS|CONSUMED) returned

[Ralph Campbell]

On Tue, 2008-05-20 at 15:17 -0700, Roland Dreier wrote:
>  >         case IB_MAD_RESULT_SUCCESS | IB_MAD_RESULT_CONSUMED:
>  >                 kmem_cache_free(ib_mad_cache, mad_priv);
>  > -               break;
>  > +               kfree(local);
>  > +               goto out;
> 
> Seems you need to set ret = 1 here?  Otherwise I think ib_post_send_mad
> will continue handling the send even though the packet was supposedly
> consumed.

I agree.

> Also as a side note, I think handle_outgoing_dr_smp() would be clearer
> if rather than having
> 
> out:
> 	return ret;
> 
> and then doing stuff like
> 
> 	ret = -EINVAL;
> 	goto out;
> 
> the code just did "return -EINVAL;"
> 
> Maybe I'll do that cleanup for 2.6.27.

I also agree but I remember at one point we got pushback from
one of the mainline kernel developers who really wanted to see
only one return point in the code even if it meant more gotos.
I don't remember who though.