> On June 16, 2011, 4:23 p.m., Boroondas Gupte wrote:
> > indra/newview/llnearbychathandler.cpp, lines 375-382
> > <http://codereview.secondlife.com/r/341/diff/2/?file=2978#file2978line375>
> >
> >     This function's return type should be changed to bool, if it's intended 
> > for use with std::sort. (And that's how it's being used currently.)

and better yet... there's no excuse in a routine this small for multiple 
returns.
Add a local variable and use the else clause.

bool inOrder;
if ( .... test ... )
{
   inOrder = false;
}
else
{
   ... calc ...
   inOrder = v1 > v2;
}
return inOrder;

any half way decent optimizer will put that bool in register anyway, so there 
is no performance difference at all and the code is much clearer and easier to 
put breakpoints and logging into.


- Oz


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/341/#review759
-----------------------------------------------------------


On June 20, 2011, 6:55 a.m., Vadim ProductEngine wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/341/
> -----------------------------------------------------------
> 
> (Updated June 20, 2011, 6:55 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> Apparently, a nearby chat toast got somehow destroyed while still remaining 
> in the list of active toasts.
> Attempt to sort active toasts in showToastsBottom() then triggered the crash.
> 
> I don't know how to reproduce the crash, i.e. force destroying a toast in a 
> way that its onClose() method (which would remove references to the toast) 
> isn't called.
> So we'll just remove references to the toast whenever it's destroyed.
> 
> 
> This addresses bug STORM-1352.
>     http://jira.secondlife.com/browse/STORM-1352
> 
> 
> Diffs
> -----
> 
>   indra/newview/llnearbychathandler.cpp UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/341/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vadim
> 
>

_______________________________________________
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Reply via email to