[webkit-reviews] review granted: [Bug 95532] [EFL] Add proper support for navigator.onLine and associated events : [Attachment 161628] Patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Christophe Dumez
<christophe.dumez at intel.com>'s request for review:
Bug 95532: [EFL] Add proper support for navigator.onLine and associated events
https://bugs.webkit.org/show_bug.cgi?id=95532

Attachment 161628: Patch
https://bugs.webkit.org/attachment.cgi?id=161628&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=161628&action=review


> Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:49
> +    // Assume that we're offline until proven otherwise.
> +    m_isOnLine = false;

It that really wise?

> Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:60
> +	   if (!syspath || !strcmp(syspath, UdevLoopBackInterfaceSysPath))

When you are already taking assumption that UdevLoo/... is non-null, why not
use strncmp() with 2?

> Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:77
> +	 eeze_net_free(static_cast<Eeze_Net*>(data));

why different indentation

> Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:93
> +    char buffer[4096];
> +    size_t len;

why not initialize len to 4096;?

size_t len = 4096
char buffer[len]

then replace it

> Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:102
> +	       if (nlh->nlmsg_type == RTM_NEWADDR
> +		   || nlh->nlmsg_type == RTM_DELADDR) {

why not just keep that on one line?

> Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:116
> +	   int sock = ecore_main_fd_handler_fd_get(m_fdHandler);

socket?

> Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:120
> +    eeze_shutdown();

what if it is used elsewhere? (I believe it is)

> Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:147
> +    if (bind(sock, (struct sockaddr *)&addr, sizeof(addr)) == -1) {

wrong styled cast