svn commit: r236377 - head/sys/dev/vxge/vxgehal

[Bruce Evans]

On Fri, 1 Jun 2012, John Baldwin wrote:

> On Friday, June 01, 2012 2:23:42 am Eitan Adler wrote:
>> On 31 May 2012 22:13, Bruce Evans <brde at optusnet.com.au> wrote:
>>> This seems to change a style by (excessive parentheses for a normal
>>> equality test) into logic bug (assignment of dtrh instead of compariing
>>> with it).
>>
>> intentional - perhaps my commit message was poorly worded.
>>
>> The comment above says
>> 283         /*
>> 284          * restore a previously allocated dtrh at current offset and update
>> 285          * the available reserve length accordingly. If dtrh is null just
>> 286          * update the reserve length, only
>> 287          */
>>
>> and gnn confirmed that the patch as committed is correct.

Oops.

> This is why I personally loathe assignment side effects in boolean expressions
> for control flow.  I tend to write this sort of thing instead as:
>
> 	channel->dtr_arr[dtr_index].dtr = dtrh;
> 	if (dtrh != NULL) {

Except here you would have written:

 	channel->dtr_arr[dtr_index].dtr = dtrh;
 	if (dtrh == NULL)
 		return;

to avoid a large compound statement for the null case.

Bruce