Re: [2/2] iphlpapi: Use the new version of the IP_ADAPTER_UNICAST_ADDRESS structure (try 2)
On Fri, 2017-01-13 at 13:27 +0100, André Hentschel wrote:
diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c index 54f7a37..223d980 100644 --- a/dlls/iphlpapi/iphlpapi_main.c +++ b/dlls/iphlpapi/iphlpapi_main.c @@ -1065,6 +1065,13 @@ static ULONG adapterAddressesFromIndex(ULONG family, ULONG flags, IF_INDEX index debugstr_ipv4(&sa->sin_addr.S_un.S_addr, addr_buf)); fill_unicast_addr_data(aa, ua);
ua->OnLinkPrefixLength = 0;for (j = 0; j < sizeof(*v4masks) * 8; j++){if (v4masks[i] & 1 << j) ua->OnLinkPrefixLength++;else break;}
Please add a helper to compute prefix length instead of duplicating this code. This should probably wait until the code freeze is over.
On 13 January 2017 at 14:43, Hans Leidekker hans@codeweavers.com wrote:
On Fri, 2017-01-13 at 13:27 +0100, André Hentschel wrote:
diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c index 54f7a37..223d980 100644 --- a/dlls/iphlpapi/iphlpapi_main.c +++ b/dlls/iphlpapi/iphlpapi_main.c @@ -1065,6 +1065,13 @@ static ULONG adapterAddressesFromIndex(ULONG family, ULONG flags, IF_INDEX index debugstr_ipv4(&sa->sin_addr.S_un.S_addr, addr_buf)); fill_unicast_addr_data(aa, ua);
ua->OnLinkPrefixLength = 0;for (j = 0; j < sizeof(*v4masks) * 8; j++){if (v4masks[i] & 1 << j) ua->OnLinkPrefixLength++;else break;}Please add a helper to compute prefix length instead of duplicating this code. This should probably wait until the code freeze is over.
For what it's worth, if you're adding a helper function anyway, note that you're really just counting the number of trailing zero-bits. GCC has __builtin_ctz() for that, but even without that there are better approaches than testing each bit individually. Alternatively, there's also __builtin_popcount().
Am 13.01.2017 um 15:34 schrieb Henri Verbeet:
On 13 January 2017 at 14:43, Hans Leidekker hans@codeweavers.com wrote:
On Fri, 2017-01-13 at 13:27 +0100, André Hentschel wrote:
diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c index 54f7a37..223d980 100644 --- a/dlls/iphlpapi/iphlpapi_main.c +++ b/dlls/iphlpapi/iphlpapi_main.c @@ -1065,6 +1065,13 @@ static ULONG adapterAddressesFromIndex(ULONG family, ULONG flags, IF_INDEX index debugstr_ipv4(&sa->sin_addr.S_un.S_addr, addr_buf)); fill_unicast_addr_data(aa, ua);
ua->OnLinkPrefixLength = 0;for (j = 0; j < sizeof(*v4masks) * 8; j++){if (v4masks[i] & 1 << j) ua->OnLinkPrefixLength++;else break;}Please add a helper to compute prefix length instead of duplicating this code. This should probably wait until the code freeze is over.
For what it's worth, if you're adding a helper function anyway, note that you're really just counting the number of trailing zero-bits. GCC has __builtin_ctz() for that, but even without that there are better approaches than testing each bit individually. Alternatively, there's also __builtin_popcount().
Thank you both for the hints. One thing is that popcount would count all 1s even with 0s in between, whereas this function stops at the first 0. Hans, you wrote it that way, there's no checking if the mask is mangled (0s in between), how should it react to mangled masks?
On Fri, 2017-01-13 at 15:52 +0100, André Hentschel wrote:
Please add a helper to compute prefix length instead of duplicating this code. This should probably wait until the code freeze is over.
For what it's worth, if you're adding a helper function anyway, note that you're really just counting the number of trailing zero-bits. GCC has __builtin_ctz() for that, but even without that there are better approaches than testing each bit individually. Alternatively, there's also __builtin_popcount().
Thank you both for the hints. One thing is that popcount would count all 1s even with 0s in between, whereas this function stops at the first 0. Hans, you wrote it that way, there's no checking if the mask is mangled (0s in between), how should it react to mangled masks?
I think we should just trust the OS to give us a valid mask.
On 13 January 2017 at 15:52, André Hentschel nerv@dawncrow.de wrote:
Thank you both for the hints. One thing is that popcount would count all 1s even with 0s in between, whereas this function stops at the first 0.
Yeah, the strict equivalent would be something along the lines of "__builtin_clz(~mask)". But as you mentioned, there's the question what should happen with broken/special masks.
Am 13.01.2017 um 16:02 schrieb Henri Verbeet:
On 13 January 2017 at 15:52, André Hentschel nerv@dawncrow.de wrote:
Thank you both for the hints. One thing is that popcount would count all 1s even with 0s in between, whereas this function stops at the first 0.
Yeah, the strict equivalent would be something along the lines of "__builtin_clz(~mask)". But as you mentioned, there's the question what should happen with broken/special masks.
I would suggest to add those functions, any objections?
static DWORD mask_v4_to_prefix(DWORD m) { #ifdef HAVE___BUILTIN_POPCOUNT return __builtin_popcount(m); #else m -= m >> 1 & 0m55555555; m = (m & 0m33333333) + (m >> 2 & 0m33333333); return ((m + (m >> 4)) & 0m0f0f0f0f) * 0m01010101 >> 24; #endif }
static DWORD mask_v6_to_prefix(SOCKET_ADDRESS *m) { const IN6_ADDR *mask = &((struct WS_sockaddr_in6 *)m->lpSockaddr)->sin6_addr; DWORD ret = 0, i;
for (i = 0; i < 8; i++) ret += mask_v4_to_prefix(mask->u.Word[i]); return ret; }
On 16 January 2017 at 18:06, André Hentschel nerv@dawncrow.de wrote:
m = (m & 0m33333333) + (m >> 2 & 0m33333333);
I think you may have replaced a few x's too much.
-
André Hentschel -
Hans Leidekker -
Henri Verbeet