Date: Sat, 2 Jun 2012 02:04:16 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: John Baldwin <jhb@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Eitan Adler <eadler@FreeBSD.org>, Bruce Evans <brde@optusnet.com.au> Subject: Re: svn commit: r236377 - head/sys/dev/vxge/vxgehal Message-ID: <20120602015640.P3206@besplex.bde.org> In-Reply-To: <201206011024.49122.jhb@freebsd.org> References: <201206010423.q514NKtf083043@svn.freebsd.org> <20120601150242.V1226@besplex.bde.org> <CAF6rxg=MWEDPVND8rZKM72P-1Z9GG%2Bqn99qHhbCNRwmFG=3buA@mail.gmail.com> <201206011024.49122.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120602015640.P3206>