[Webkit-unassigned] [Bug 51364] Web Inspector: Remote Web Inspector - Cross Platform InspectorServer

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=51364


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #80249|review?                     |review+
               Flag|                            |




--- Comment #43 from Alexey Proskuryakov <ap at webkit.org>  2011-01-26 17:08:02 PST ---
(From update of attachment 80249)
View in context: https://bugs.webkit.org/attachment.cgi?id=80249&action=review

> Source/WebCore/ChangeLog:13
> +        didReceiveConnection callback with a generic SocketStreamHandle.

I'm not sure if "didReceiveConnection" parses. Maybe "establish"?

> Source/WebCore/platform/network/ServerSocketHandleBase.cpp:49
> +    ASSERT(m_port);
> +    if (!m_port)
> +        return;

We only do this when fixing super elusive bugs by adding semi-arbitrary null checks. If m_port can't be null, just assert, and don't make runtime checks.

> Source/WebCore/platform/network/ServerSocketHandleClient.h:45
> +} // namespace WebCore

I'm not a fan of such comments - if you really need to check what this brace at the end of the file is for, you'll double-click it for IDE to balance, and if you're not, why is this boilerplate asking for attention?

> Source/WebCore/platform/network/cf/ServerSocketHandleCFNet.cpp:58
> +    m_serverFileDescriptor = socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP);

We don't have CF level APIs to create listening sockets? This whole function feels alien, with perror and such.

> Source/WebCore/platform/network/cf/ServerSocketHandleCFNet.cpp:70
> +    sockaddr6.sin6_addr = in6addr_any;

Should there be any limit on which interfaces to bind to? Even when running regression tests, we only listen on loopback, because opening up on all interfaces isn't prudent security wise.

> Source/WebCore/platform/network/cf/ServerSocketHandleCFNet.cpp:157
> +    int fd = *(const int*)data;

I don't think that const has any real meaning here.

Please use a C++ style cast.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.