[openib-general] Re: [PATCH] sdp conditional code cleanup

[Libor Michalek]

On Mon, Feb 28, 2005 at 11:34:06PM +0200, Michael S. Tsirkin wrote:
> Quoting r. Libor Michalek <libor at topspin.com>:
> > Subject: Re: [PATCH] sdp conditional code cleanup
> > 
> > On Sun, Feb 27, 2005 at 12:44:41PM +0200, Michael S. Tsirkin wrote:
> > > Quoting r. Libor Michalek <libor at topspin.com>:
> > > > On Thu, Feb 24, 2005 at 11:49:28PM +0200, Michael S. Tsirkin wrote:
> > > > > OK, now what about things like these:
> > > > > 
> > > > >         if (0 > result) {
> > > > > 
> > > > > may I change them all to
> > > > > 
> > > > >         if (result < 0) {
> > > > > 
> > > > > While being equivalent, we are testing the result, not 0.
> > > > > 
> > > > > Similiarly (although I feel somewhat less strongly about it)
> > > > > 
> > > > >         if (0 == result)
> > > > > and
> > > > >         if (NULL == conn)
> > > > > 
> > > > > are better off as
> > > > > 
> > > > > 	if (!result) {
> > > > > and
> > > > >         if (!conn)
> > > > > 
> > > > > C is a Spartan language, and this is more brief.
> > > > > Libor, I think I asked about the second one, but dont recall you
> > > > > answering.
> > > > > If OK to both, let me know and I'll do it on Sunday.
> > > > 
> > > >   I actually feel more strongly in favour of making the second change
> > > > you propose then the first. However, I'm OK with both, so feel free
> > > > to submit a patch.
> > > 
> > > Here is the patch.
> > > 
> > > I generalized the approach - 
> > > any if (CONSTANT == variable) and if ( CONSTANT & variable) is now
> > >     if (variable == CONSTANT) and if ( variable & CONSTANT)
> > >     and so forth.
> > > 
> > > Further, some places had weird flag testing code like this:
> > > if ( (variable & CONSTANT) > 0 )
> > > 
> > > I changed them all to 
> > > 
> > > if (variable & CONSTANT)
> > > 
> > > Two things I noticed but did not fix:
> > > 
> > > A. In some places a positive error code is returned. For example
> > >    ENOBUF and not -ENOBUF. I assume its a bug but did not touch it.
> > 
> >   I'm glad you did not change this, it's not a bug. In parts of both
> > the send and recv data paths positive and negative returns have different
> > meaning. negative return is a hard error, while positive return is used
> > to indicate that the data path is in a flow control situation and that
> > the action will need to be retried later. Zero as usual means success.
> > 
> > > Looks like a lot of changes, I went over them several times and
> > > everything looks in order to me. I really hope its applied before
> > > any other change, it almost surely will conflict with any other
> > > patch, and it'll be a lot of work to re-diff.
> > 
> >   I found a few problem spots that I fixed up, before commiting
> > the code. The third problem was in an #if 0, so it wasn't an
> > immediate issue. Thanks.
> 
> Here is a small number of additional swaps I missed on the first pass.

  Thanks, applied and commited.

-Libor