[PATCH 1/1] ws2_32: Selectively bind UDP sockets to interfaces while still allowing broadcast packets (try 2, resend).

[Juan Lang]

Hi Erich,

+ * Bind the given socket exclusively to a specific interface.

I know the style around here is to comment minimally, but I didn't
find it clear what the behavior of this function is.  For one thing,

+static int interface_bind( int fd, struct sockaddr *addr )
(snip)
+    int ret = FALSE;
(snip)
+                ret = TRUE;

You're using #defines used with BOOL, so a BOOL return value would be
a little clearer IMO.  int, in the context of socket functions, often
uses 0 => success, -1 => failure.

Still, even with that change,

+    if (in_sock->sin_addr.s_addr == htonl(WS_INADDR_ANY))
+    {
+        /* Not binding to specific interface, use default route */
+        goto cleanup;
+    }
+    optlen = sizeof(sock_type);
+    if (getsockopt(fd, SOL_SOCKET, SO_TYPE, &sock_type, &optlen) == -1
+        || sock_type != SOCK_DGRAM)
+    {
+        /* Not a UDP socket, interface-specific bind is irrelevant. */
+        goto cleanup;

These two blocks will use the default return value, FALSE, and it
wasn't clear to me from the function name or comment that it should
return FALSE in this case.  Not that doing so is incorrect:  just that
it isn't obvious that that's the way it should behave.  In this case,
I feel a comment indicating that the function returns TRUE iff the
function bound to a specific device, and that FALSE doesn't
necessarily mean failure, is merited.

Also,

+    /* Obtain the size of the IPv4 adapter list and allocate
structure memory */
+    if (GetAdaptersInfo(NULL, &adap_size) != ERROR_BUFFER_OVERFLOW)

Why do all this outside the #ifdef SO_BINDTODEVICE guard?  I'd rather
you just had a different implementation of the function when
SO_BINDTODEVICE isn't present that issues the warning and does nothing
else.

I'll let Alexandre comment on whether the approach itself is appropriate.

Thanks,
--Juan