[opensource-dev] Review Request: VWR-24333: Hardening against use of getLindenUserDir() before logging in.

[Boroondas Gupte]

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



indra/llvfs/lldir.cpp
<http://codereview.secondlife.com/r/91/#comment272>

    I know we already have a lot of methods taking boolean arguments, but it's probably worth mentioning http://doc.qt.nokia.com/qq/qq13-apis.html#thebooleanparametertrap here. (I.e., boolean arguments make the calls of the method harder to read, when the method name doesn't already imply the semantics of the argument. You can give the reader a hint towards the semantics by using nicely named enums instead.)



indra/newview/llappviewer.cpp
<http://codereview.secondlife.com/r/91/#comment273>

    E.g. here, it's impossible to guess what the "true" indicates. Either you already know, or you have to look it up.


- Boroondas


On Jan. 14, 2011, 1:05 p.m., Aleric Inglewood wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/91/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2011, 1:05 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> Without this patch, getLindenUserDir() is sometimes used without
> checking if it returns an empty value, resulting in trying to open
> an empty file and then gracefully recovering from that. So, this
> patch doesn't really fix a bug. However it might prevent one in the
> future. Note that this DID lead to a bug in Viewer 1 code, so it's
> possible. The main threat is that some singleton class that uses
> getLindenUserDir (indirectly) is instantiated before logging in:
> A singleton is only created once and when it is initialized with
> an empty getLindenUserDir() then that can remain.
> 
> This patch aborts when the viewer is compiled in debug mode (not
> in release mode, when it will do what it did before this patch)
> and when getLindenUserDir() is called before it was initialized,
> unless the developer explicitely passes 'true' (empty_ok) to
> getLindenUserDir, signaling that they considered the possibility
> that the function is being called before the user logged in.
> 
> 
> This addresses bug VWR-24333.
>     http://jira.secondlife.com/browse/VWR-24333
> 
> 
> Diffs
> -----
> 
>   indra/llvfs/lldir.h 422f636c3343 
>   indra/llvfs/lldir.cpp 422f636c3343 
>   indra/newview/llappviewer.cpp 422f636c3343 
>   indra/newview/llbottomtray.cpp 422f636c3343 
>   indra/newview/llfilepicker.cpp 422f636c3343 
>   indra/newview/llnavigationbar.cpp 422f636c3343 
>   indra/newview/llsearchhistory.h 422f636c3343 
>   indra/newview/llurlhistory.cpp 422f636c3343 
>   indra/newview/llviewerinventory.cpp 422f636c3343 
>   indra/newview/llviewermedia.cpp 422f636c3343 
>   indra/newview/llvoiceclient.cpp 422f636c3343 
> 
> Diff: http://codereview.secondlife.com/r/91/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Aleric
> 
>

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