GNOME Bugzilla – Bug 697185
GSocket – Allow specifying the multicast interface from an IPv4 address
Last modified: 2013-08-19 16:29:42 UTC
Created attachment 240492 [details] [review] 0001-GSocket-Allow-specifying-the-multicast-interface-fro.patch Hi, the attached patch allows specifying the multicast interface from an IPv4 address This is the only way to specify an interface on Windows before XP. If the passed string is not a valid IPv4 address it will be handled as an interface name instead.
Comment on attachment 240492 [details] [review] 0001-GSocket-Allow-specifying-the-multicast-interface-fro.patch >This is the only way to specify an interface on Windows before XP. Don't we require at least Windows XP now? From configure.ac: AC_DEFINE([_WIN32_WINNT], [0x0501], [Target the Windows XP API]) I forget the specifics, but I'm pretty sure we non-conditionally depend on certain functions that only exist in XP now... But anyway (in case that's not true), I don't like this change, because the point of having glib-level APIs is to abstract around stuff like this; we don't want people to have to use the API differently if they want to support old Winodws. Can't glib just translate the interface name to an IP address itself in this case? It looks like you ought to be able to do that by using GetAdaptersInfo() (http://msdn.microsoft.com/en-us/library/windows/desktop/aa365917%28v=vs.85%29.aspx). (There might be a simpler method in there somewhere too; I didn't look that hard.)
(In reply to comment #1) > (From update of attachment 240492 [details] [review]) > >This is the only way to specify an interface on Windows before XP. > > Don't we require at least Windows XP now? From configure.ac: > > AC_DEFINE([_WIN32_WINNT], [0x0501], [Target the Windows XP API]) > > I forget the specifics, but I'm pretty sure we non-conditionally depend on > certain functions that only exist in XP now... Erm, typo. That's for Windows XP and earlier versions, so effectively Windows XP only :) > But anyway (in case that's not true), I don't like this change, because the > point of having glib-level APIs is to abstract around stuff like this; we don't > want people to have to use the API differently if they want to support old > Winodws. > > Can't glib just translate the interface name to an IP address itself in this > case? It looks like you ought to be able to do that by using GetAdaptersInfo() > (http://msdn.microsoft.com/en-us/library/windows/desktop/aa365917%28v=vs.85%29.aspx). > (There might be a simpler method in there somewhere too; I didn't look that > hard.) Ah, I missed that function... everything I looked up was only available after Windows XP. So yes, makes sense to change it to that instead. How would you handle the case where an interface has multiple addresses assigned? Join the multicast group for all of these addresses?
(In reply to comment #2) > How would you handle the case where an interface has multiple addresses > assigned? Join the multicast group for all of these addresses? Oh, hey, the documentation for struct ip_mreq (http://msdn.microsoft.com/en-us/library/windows/desktop/ms738695%28v=vs.85%29.aspx) says that you can actually put an ifindex (in network byte order) in the imr_interface field instead of an address. So I guess basically you'll just be using GetAdaptersInfo() to implement if_nametoindex().
Ah, that indeed looks like the best option for this. Thanks :) I hope I'll have some time to implement this soon
Created attachment 250542 [details] [review] 0001-GSocket-Implement-multicast-interface-selection-on-W.patch Untested patch that implements this. It compiles at least. Please review, I'll test it later
Comment on attachment 250542 [details] [review] 0001-GSocket-Implement-multicast-interface-selection-on-W.patch basically looks good >+#ifdef G_OS_WIN32 >+# include <iphlpapi.h> >+#endif put that with the other networking includes in gnetworking.h.in >+ gulong addresses_len = 15*1024; the docs say you can just pass a NULL addresses pointer to GetAdaptersAddresses() to get back the right addresses_len. So you don't need to initialize the buffer to a random size. >+ addresses = g_realloc (addresses, addresses_len); >+ >+ if (!addresses) >+ return 0; g_realloc() cannot fail >+ res = GetAdaptersAddresses(AF_UNSPEC, 0, NULL, addresses, &addresses_len); space before "(" >+ if (i == 4 || res != NO_ERROR) >+ { >+ g_free (addresses); >+ return 0; >+ } although not needed in any of the places where if_nametoindex() is *currently* used in gsocket.c, you should probably set errno on error anyway.
Created attachment 251057 [details] [review] 0002-GSocket-Implement-multicast-interface-selection-on-W.patch
Created attachment 251061 [details] [review] 0002-GSocket-Implement-multicast-interface-selection-on-W.patch
pushed