[ofw] patch: [IBAL] remove unnecessary ASSERT

[Fab Tillier]

Ugh, seriously?  Thanks for giving time to discuss this.

The switch statement handles all possible pd_types.  Are you saying that your unit tests passes in arbitrary values for PD types?

It seems like Mellanox is on some quest to just sync the OFED source with whatever you have in-house, regardless of the feedback.  While that's a nice idea, you shouldn't just ram whatever your changes are down the OFED community's throat.  A better model would be to do all your work in the OFED tree, for everyone to see and comment on while you do it.

This has come up over and over again over many years: you do a bunch of coding behind closed doors, and then dump everything into the OFED source tree - too much code to be reviewed, too much time invested on your part to bother addressing the community's comments when there are some.  Please start treating the community as partners, not subordinates.

-Fab

From: Tzachi Dar [mailto:tzachid at mellanox.co.il]
Sent: Thursday, September 02, 2010 12:23 AM
To: Leonid Keller; Fab Tillier; ofw at lists.openfabrics.org
Subject: RE: patch: [IBAL] remove unnecessary ASSERT

Applied on 2887.

Thanks
Tzachi

From: Leonid Keller
Sent: Wednesday, September 01, 2010 12:54 PM
To: Fab Tillier; Tzachi Dar; ofw at lists.openfabrics.org
Subject: RE: patch: [IBAL] remove unnecessary ASSERT

We hit it in our test program, which - in error flow - creates PD with all possible pd_types.
The test expects to get IB_INVALID_PARAMETER, which it gets.
We believe, that the error status that is returned in this case is enough to spot an error.

From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Fab Tillier
Sent: Tuesday, August 31, 2010 7:56 PM
To: Tzachi Dar; ofw at lists.openfabrics.org
Subject: Re: [ofw] patch: [IBAL] remove unnecessary ASSERT

The assert here is to trap a bad call to alloc_pd.  Why is it being removed?  Did you have code that hit it?  Did you fix that code so it wouldn't hit the assertion?  Or was the assertion incorrect?  Basically, the assert has been in the code for years now - what suddenly makes it unnecessary?

-Fab

From: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Tzachi Dar
Sent: Tuesday, August 31, 2010 8:29 AM
To: ofw at lists.openfabrics.org
Subject: [ofw] patch: [IBAL] remove unnecessary ASSERT

Signed off by: Leonid

Mlnx 5599

Index: B:/users/tzachid/projinf9/trunk/core/al/al_pd.c
===================================================================
--- B:/users/tzachid/projinf9/trunk/core/al/al_pd.c         (revision 5598)
+++ B:/users/tzachid/projinf9/trunk/core/al/al_pd.c      (revision 5599)
@@ -137,7 +137,6 @@
                                break;

                default:
-                              CL_ASSERT( h_pd->type == IB_PDT_ALIAS || h_pd->type == IB_PDT_NORMAL );
                                status = IB_INVALID_PARAMETER;
                }

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20100902/35d255ef/attachment.html>