----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/374/#review1005 -----------------------------------------------------------
indra/llcommon/llsingleton.h <http://codereview.secondlife.com/r/374/#comment1018> why are we getting the data from within the destructor of the singleton itself? Why not just do if (mInitState != DELETED) instead? Seems like it would be less confusing. indra/llmessage/llcurl.h <http://codereview.secondlife.com/r/374/#comment1019> maybe a little comment as to what "Easy" is in this context. I know you just moved the code to the header, but now that it is more than an implementation detail of LLCurl, I think our expectation of documentation has increased. indra/llmessage/llcurl.cpp <http://codereview.secondlife.com/r/374/#comment1020> if time_out is unused now, let's remove it from the signature indra/llmessage/llproxy.h <http://codereview.secondlife.com/r/374/#comment1021> do all these #defines and socks structs need to be in the header? indra/llmessage/llproxy.h <http://codereview.secondlife.com/r/374/#comment1022> I think we need something a bit more obvious to call out locking methods, maybe just a heavier comment header? indra/llmessage/llproxy.cpp <http://codereview.secondlife.com/r/374/#comment1025> control-flow wise, it might be clearer to move the final check for status to a separate if, with appropriate comment along the lines of "if any of the above failed..." indra/llmessage/llproxy.cpp <http://codereview.secondlife.com/r/374/#comment1026> can we reflect the potentially long blocking period in the name of the method? tcp_wait_for_handshake or similar? - Richard On Aug. 25, 2011, 11:15 a.m., Log Linden wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/374/ > ----------------------------------------------------------- > > (Updated Aug. 25, 2011, 11:15 a.m.) > > > Review request for Viewer, Oz Linden, Monty Brandenberg, and Stone Linden. > > > Summary > ------- > > This is a continuation of Robin Cornelius's SOCKS 5 contribution, shown in > https://codereview.secondlife.com/r/232/ . I have tried to address all of > the comments on that code review and do as much cleanup as possible. The diff > includes everything that was submitted by Robin, as well as my work. > Major changes since I started working: > * Changed SOCKS 5 proxy control channel to use the existing LLSocket class, > which is a thin wrapper around APR sockets. > * Worked with the Linden Lab UX team to revamp the proxy controls. > * Proxy credentials are now stored in the LLSecAPI password storage, which is > the same that is used for users' Second Life Credentials instead of as being > stored in the clear as a preference. > > > This addresses bug STORM-1112. > http://jira.secondlife.com/browse/STORM-1112 > > > Diffs > ----- > > indra/llcommon/llerror.h 4ebbd04efd93 > indra/llcommon/llerror.cpp 4ebbd04efd93 > indra/llcommon/llsingleton.h 4ebbd04efd93 > indra/llmessage/CMakeLists.txt 4ebbd04efd93 > indra/llmessage/llcurl.h 4ebbd04efd93 > indra/llmessage/llcurl.cpp 4ebbd04efd93 > indra/llmessage/llhttpassetstorage.cpp 4ebbd04efd93 > indra/llmessage/llhttpclient.cpp 4ebbd04efd93 > indra/llmessage/lliosocket.h 4ebbd04efd93 > indra/llmessage/lliosocket.cpp 4ebbd04efd93 > indra/llmessage/llpacketring.h 4ebbd04efd93 > indra/llmessage/llpacketring.cpp 4ebbd04efd93 > indra/llmessage/llproxy.h PRE-CREATION > indra/llmessage/llproxy.cpp PRE-CREATION > indra/llmessage/llurlrequest.cpp 4ebbd04efd93 > indra/llmessage/net.h 4ebbd04efd93 > indra/llmessage/net.cpp 4ebbd04efd93 > indra/llui/llfunctorregistry.h 4ebbd04efd93 > indra/newview/app_settings/settings.xml 4ebbd04efd93 > indra/newview/llappviewer.cpp 4ebbd04efd93 > indra/newview/llfloaterpreference.h 4ebbd04efd93 > indra/newview/llfloaterpreference.cpp 4ebbd04efd93 > indra/newview/llloginhandler.cpp 4ebbd04efd93 > indra/newview/llpanellogin.h 4ebbd04efd93 > indra/newview/llsecapi.h 4ebbd04efd93 > indra/newview/llstartup.h 4ebbd04efd93 > indra/newview/llstartup.cpp 4ebbd04efd93 > indra/newview/llviewerfloaterreg.cpp 4ebbd04efd93 > indra/newview/llxmlrpctransaction.cpp 4ebbd04efd93 > indra/newview/skins/default/xui/en/floater_preferences_proxy.xml > PRE-CREATION > indra/newview/skins/default/xui/en/notifications.xml 4ebbd04efd93 > indra/newview/skins/default/xui/en/panel_cof_wearables.xml 4ebbd04efd93 > indra/newview/skins/default/xui/en/panel_preferences_privacy.xml > 4ebbd04efd93 > indra/newview/skins/default/xui/en/panel_preferences_setup.xml 4ebbd04efd93 > > Diff: http://codereview.secondlife.com/r/374/diff > > > Testing > ------- > > I've tested exclusively on Linux so far. I'm working on a more extensive > test plan that includes setting up a gateway with a restrictive firewall to > verify that all traffic is going through the proxy. > > Test builds and screenshots of the changed UI elements are available from the > project page, located here: > https://wiki.secondlife.com/wiki/User:Log_Linden/Socks5Viewer > > > Thanks, > > Log > >
_______________________________________________ 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
