[opensource-dev] Review Request: VWR-25608 error on shutdown due to buffer overrun in LLVFS::audit

[Boroondas Gupte]

> On April 27, 2011, 3:30 a.m., Boroondas Gupte wrote:
> > indra/llvfs/llvfs.cpp, lines 1716-1717
> > <http://codereview.secondlife.com/r/278/diff/1/?file=1511#file1511line1716>
> >
> >     When index_size == 0, wouldn't it be more appropriate to skip the steps that require taking the address of element 0? They'd be nil operations in that case, anyway, wouldn't they?
> 
> Brad Kittenbrink wrote:
>     In theory, you're right, but my intent was to make the minimally invasive change that would fix the error, and be sure to preserve all existing behaviors.  I was hesitant to engage in any more extensive refactoring without a good plan for how we want to test this subsystem.

Fair enough.


On April 27, 2011, 3:30 a.m., Brad Kittenbrink wrote:
> > Finally, if we are only ever accessing the underlying memory directly (as seems to be the case here), why use a std::vector as buffer instead of an array?
> 
> Brad Kittenbrink wrote:
>     I can't speak to the intent of the original author of this code, but I always use vectors for dynamically sized arrays, as they automatically free the buffer.  Using new[] or malloc is far more error prone, even when using std::auto_ptr or boost::scoped_ptr for RAII.

Ah, right ... I keep forgetting that constant sized isn't enough for being statically sized.


- Boroondas


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


On April 26, 2011, 5:31 p.m., Brad Kittenbrink wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/278/
> -----------------------------------------------------------
> 
> (Updated April 26, 2011, 5:31 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> Fix for a minor buffer overrun on shutdown in LLVFS::audit.
> 
> 
> This addresses bug VWR-25608.
>     http://jira.secondlife.com/browse/VWR-25608
> 
> 
> Diffs
> -----
> 
>   indra/llvfs/llvfs.cpp UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/278/diff
> 
> 
> Testing
> -------
> 
> I tested using the Microsoft Debug Heap and confirmed that this allows the Debug Heap to shut down without errors.
> 
> 
> Thanks,
> 
> Brad
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110427/a40142e4/attachment.htm