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



indra/llmessage/llcurl.cpp
<http://codereview.secondlife.com/r/374/#comment880>

    Correct whitespace.



indra/llmessage/llpacketring.h
<http://codereview.secondlife.com/r/374/#comment884>

    A few things mostly on coding style here:
    1.  You'll mostly see method declarations separate from member 
declarations.  It's 
    often due to access controls but it's also
    a good organizational thing.  I'd move the method up here.
    2.  This is really a private/protected method.  No external user should 
call this.
    Making it so is compatible with 1.
    3.  Terrible name.  SendPacket and doSendPacket.  This is some sort of
    sendPacketImpl or routePacket, I think.



indra/llmessage/llpacketring.cpp
<http://codereview.secondlife.com/r/374/#comment890>

    With the proxy code, we could get data
    larger than NET_BUFFER_SIZE.  We don't
    detect this condition in receive_packet.
    Might just log initially.  So much wrong
    with it....



indra/llmessage/llpacketring.cpp
<http://codereview.secondlife.com/r/374/#comment886>

    Magic number from original code review.
    Need these definitions (10) in the socks
    protocol definition set.



indra/llmessage/llpacketring.cpp
<http://codereview.secondlife.com/r/374/#comment888>

    Not necessary now but something for a
    follow-on jira.  This would have been a 
    good place for a scatter/gather i/o using
    recvmsg to avoid the data copying.



indra/llmessage/llpacketring.cpp
<http://codereview.secondlife.com/r/374/#comment891>

    Isn't packet_size '10' too large at this point?



indra/llmessage/llpacketring.cpp
<http://codereview.secondlife.com/r/374/#comment889>

    I just looked into get_sender() and 
    get_receiving_interface().  *weep*  They
    use statics.  So no thread safety.  If we
    do improve the receive_packet interface to
    ever to scatter/gather, *this* will go.



indra/llmessage/llpacketring.cpp
<http://codereview.secondlife.com/r/374/#comment892>

    Another perfect opportunity for scatter/gather i/o.



indra/llmessage/llpacketring.cpp
<http://codereview.secondlife.com/r/374/#comment893>

    Caller may still reasonably try to write
    a full NET_BUFFER_SIZE of data in which case
    this smashes memory.
    


- Monty


On July 12, 2011, 11:20 a.m., Log Linden wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/374/
> -----------------------------------------------------------
> 
> (Updated July 12, 2011, 11:20 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/llmessage/CMakeLists.txt c7a4b7a24e05 
>   indra/llmessage/llcurl.cpp c7a4b7a24e05 
>   indra/llmessage/lliosocket.h c7a4b7a24e05 
>   indra/llmessage/lliosocket.cpp c7a4b7a24e05 
>   indra/llmessage/llpacketring.h c7a4b7a24e05 
>   indra/llmessage/llpacketring.cpp c7a4b7a24e05 
>   indra/llmessage/llproxy.h PRE-CREATION 
>   indra/llmessage/llproxy.cpp PRE-CREATION 
>   indra/llmessage/net.h c7a4b7a24e05 
>   indra/llmessage/net.cpp c7a4b7a24e05 
>   indra/llui/llfunctorregistry.h c7a4b7a24e05 
>   indra/newview/app_settings/settings.xml c7a4b7a24e05 
>   indra/newview/llappviewer.cpp c7a4b7a24e05 
>   indra/newview/llfloaterpreference.h c7a4b7a24e05 
>   indra/newview/llfloaterpreference.cpp c7a4b7a24e05 
>   indra/newview/llloginhandler.cpp c7a4b7a24e05 
>   indra/newview/llpanellogin.h c7a4b7a24e05 
>   indra/newview/llsecapi.h c7a4b7a24e05 
>   indra/newview/llstartup.h c7a4b7a24e05 
>   indra/newview/llstartup.cpp c7a4b7a24e05 
>   indra/newview/llviewerfloaterreg.cpp c7a4b7a24e05 
>   indra/newview/llxmlrpctransaction.cpp c7a4b7a24e05 
>   indra/newview/skins/default/xui/en/floater_preferences_proxy.xml 
> PRE-CREATION 
>   indra/newview/skins/default/xui/en/notifications.xml c7a4b7a24e05 
>   indra/newview/skins/default/xui/en/panel_cof_wearables.xml c7a4b7a24e05 
>   indra/newview/skins/default/xui/en/panel_preferences_privacy.xml 
> c7a4b7a24e05 
>   indra/newview/skins/default/xui/en/panel_preferences_setup.xml c7a4b7a24e05 
> 
> 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

Reply via email to