On Tue, Nov 24, 2015 at 08:30:13PM +0800, Bruno Jesus wrote:
Hi, Jamie. Thanks for your effort, as you may have observed in the patches list your patch is marked as deferred because we are in code freeze for Wine 1.8 release. This code was touched a few times in attempts to fix this same issue you are addressing so maybe this time it will be the definitive fix =)
Hello, Bruno. Thanks for your review.
I think that a true definitive fix would include removing the magic loopback address completely, as it's obviously a hack (and not a value that would ever be returned by Windows). That seems like a much more invasive change.
I have a similar (not so robust) old patch that I used on FreeBSD (and also works in Linux), could you try it in OSX just out of curiosity, please? It has printf to help showing the results so a make test in the ws2_32/tests folder is enough to check if it works.
Yes, that patch works for me. The main difference is that it includes the magic loopback address in the returned list if there are any routes to a loopback interface in the forwarding table (actually, one copy of the magic loopback address per loopback interface, I think... but there's generally only one loopback interface). And it doesn't return all of the addresses from an interface (although I've never actually seen one have more than one address - but the data structure supports it).
Is the resulting list of addresses supposed to contain loopback addresses? (I.e., does it on Windows? I don't have a Windows machine on which to test.) If so, then the patch that I submitted is deficient in that regard, as it will never return addresses from loopback interfaces. (Of course, the existing implementation and your patch will both return the magic loopback address rather than any addresses actually assigned to the interface, but that's a different problem.)
In any case, either patch will solve the specific problem that I was trying to solve (getting the correct IP address displayed when hosting a Diablo 2 game), so in that respect I don't care which one makes it into a release. (In fact, now that I have a Wineskin with a custom wine build that works for me, I don't care if it ever makes it into a release - but I assume other people who don't have the ability to fix it themselves would like it, which is why I submitted a patch in the first place.)
About your patch I just would like to say that you should follow the surrounding code style, so the { goes in the next line. I tested it on Linux and it works for Heroes of Might & Magic 2 (https://bugs.winehq.org/show_bug.cgi?id=22819).
I was following the style used by 4 of the 7 opening braces in WS_gethostbyname (for which WS_get_local_ips is a helper, and which appears immediately following WS_get_local_ips in socket.c). Perhaps someone should change the 10% of the open braces in this file that do not appear alone on their lines if they are all meant to use a consistent style?
And speaking of style, I suppose the indentation should be 4 spaces rather than whatever default my editor used?
Jamie