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

[Boroondas Gupte]

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



indra/llvfs/llvfs.cpp
<http://codereview.secondlife.com/r/278/#comment614>

    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?



indra/llvfs/llvfs.cpp
<http://codereview.secondlife.com/r/278/#comment615>

    e.g. here we could extend the condition to
    	if (!buffer.empty() && (fread(&buffer[0], 1, index_size, mIndexFP) != index_size))



indra/llvfs/llvfs.cpp
<http://codereview.secondlife.com/r/278/#comment616>

    If index_size == 0, we don't even enter this loop ...



indra/llvfs/llvfs.cpp
<http://codereview.secondlife.com/r/278/#comment617>

    ... so taking the address here shouldn't be problematic.


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?

- Boroondas


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/53867675/attachment-0001.htm