[PATCH v3] iphlpapi: Add support for ConvertLengthToIpv4Mask()

[Dagfinn Reiakvam]

On 19. mars 2018 13:40, Huw Davies wrote:
> On Sun, Mar 18, 2018 at 09:40:46PM +0100, Dagfinn Reiakvam wrote:
>> From: Uberdaff <dagfinn at reiakvam.no>
> I'm not sure why you're including the From: line, simply leave it out
> unless you're sending a patch from somebody else.
>
>> index 97284ed..62437e0 100644
>> --- a/dlls/iphlpapi/iphlpapi_main.c
>> +++ b/dlls/iphlpapi/iphlpapi_main.c
>> @@ -3223,6 +3223,30 @@ DWORD WINAPI ConvertInterfaceNameToLuidW(const WCHAR *name, NET_LUID *luid)
>>   }
>>   
>>   /******************************************************************
>> + *    ConvertLengthToIpv4Mask (IPHLPAPI.@)
>> + */
>> +DWORD WINAPI ConvertLengthToIpv4Mask(ULONG mask_len, PULONG mask)
>> +{
>> +    if(mask_len > 32)
>> +    {
>> +        *mask = INADDR_NONE;
>> +        return ERROR_INVALID_PARAMETER;
>> +    }
>> +
>> +    if(mask_len == 0)
>> +        *mask = 0;
>> +    else
>> +    {
>> +        //IN_ADDR temp;
>> +        //temp.S_un.S_addr = 0xffffffff << ( 32 - MaskLength);
>> +        //*Mask = temp.S_un.S_un_b.s_b4 + ( temp.S_un.S_un_b.s_b3 <-< 8 ) + ( temp.S_un.S_un_b.s_b2 << 16 ) + ( temp.S_un.S_un_b.s_b1 << 24 );
> Why did you leave this old code in here?  Just get rid of it.  Besides
> which c++ comments are not acceptable in Wine.
>
>> +        *mask = htonl(~0u << (32 - mask_len));
>> +    }
>> +
>> +    return NO_ERROR;
>> +}
>> +
>> +/******************************************************************
>>    *    if_nametoindex (IPHLPAPI.@)
>>    */
>>   IF_INDEX WINAPI IPHLP_if_nametoindex(const char *name)
>> diff --git a/dlls/iphlpapi/tests/iphlpapi.c b/dlls/iphlpapi/tests/iphlpapi.c
>> index 873612d..039d503 100644
>> --- a/dlls/iphlpapi/tests/iphlpapi.c
>> +++ b/dlls/iphlpapi/tests/iphlpapi.c
>> @@ -96,6 +96,7 @@ static DWORD (WINAPI *pConvertInterfaceLuidToNameW)(const NET_LUID*,WCHAR*,SIZE_
>>   static DWORD (WINAPI *pConvertInterfaceLuidToNameA)(const NET_LUID*,char*,SIZE_T);
>>   static DWORD (WINAPI *pConvertInterfaceNameToLuidA)(const char*,NET_LUID*);
>>   static DWORD (WINAPI *pConvertInterfaceNameToLuidW)(const WCHAR*,NET_LUID*);
>> +static DWORD (WINAPI *pConvertLengthToIpv4Mask)(ULONG,PULONG);
>>   
>>   static PCHAR (WINAPI *pif_indextoname)(NET_IFINDEX,PCHAR);
>>   static NET_IFINDEX (WINAPI *pif_nametoindex)(const char*);
>> @@ -149,6 +150,7 @@ static void loadIPHlpApi(void)
>>       pConvertInterfaceLuidToNameW = (void *)GetProcAddress(hLibrary, "ConvertInterfaceLuidToNameW");
>>       pConvertInterfaceNameToLuidA = (void *)GetProcAddress(hLibrary, "ConvertInterfaceNameToLuidA");
>>       pConvertInterfaceNameToLuidW = (void *)GetProcAddress(hLibrary, "ConvertInterfaceNameToLuidW");
>> +    pConvertLengthToIpv4Mask = (void *)GetProcAddress(hLibrary, "ConvertLengthToIpv4Mask");
>>       pif_indextoname = (void *)GetProcAddress(hLibrary, "if_indextoname");
>>       pif_nametoindex = (void *)GetProcAddress(hLibrary, "if_nametoindex");
>>     }
>> @@ -2174,6 +2176,67 @@ static void test_GetUnicastIpAddressTable(void)
>>       pFreeMibTable(table);
>>   }
>>   
>> +static void test_ConvertLengthToIpv4Mask(void)
>> +{
>> +  DWORD apiReturn;
>> +  ULONG mask;
>> +
>> +  if (!pConvertLengthToIpv4Mask)
>> +  {
>> +      win_skip( "ConvertLengthToIpv4Mask not available\n" );
>> +      return;
>> +  }
>> +
>> +  apiReturn = pConvertLengthToIpv4Mask(0, &mask);
>> +  ok(apiReturn == NO_ERROR,
>> +   "ConvertLengthToIpv4Mask returned %d, expected NO_ERROR\n", apiReturn);
>> +  ok(mask == 0,
>> +   "ConvertLengthToIpv4Mask Mask value %x, expected  0x00000000\n", mask);
>> +
>> +  apiReturn = pConvertLengthToIpv4Mask(1, &mask);
>> +  ok(apiReturn == NO_ERROR,
>> +   "ConvertLengthToIpv4Mask returned %d, expected NO_ERROR\n", apiReturn);
>> +  ok(mask == htonl(0x80000000),
>> +   "ConvertLengthToIpv4Mask Mask value %x, expected 0x00000080\n", mask);
>> +
>> +  apiReturn = pConvertLengthToIpv4Mask(8, &mask);
>> +  ok(apiReturn == NO_ERROR,
>> +   "ConvertLengthToIpv4Mask returned %d, expected NO_ERROR\n", apiReturn);
>> +  ok(mask == htonl(0xff000000),
>> +   "ConvertLengthToIpv4Mask Mask value %x, expected 0x000000ff\n", mask);
>> +
>> +  apiReturn = pConvertLengthToIpv4Mask(16, &mask);
>> +  ok(apiReturn == NO_ERROR,
>> +   "ConvertLengthToIpv4Mask returned %d, expected NO_ERROR\n", apiReturn);
>> +  ok(mask == htonl(0xffff0000),
>> +   "ConvertLengthToIpv4Mask Mask value %x, expected 0x0000ffff\n", mask);
>> +
>> +  apiReturn = pConvertLengthToIpv4Mask(24, &mask);
>> +  ok(apiReturn == NO_ERROR,
>> +   "ConvertLengthToIpv4Mask returned %d, expected NO_ERROR\n", apiReturn);
>> +  ok(mask == htonl(0xffffff00),
>> +   "ConvertLengthToIpv4Mask Mask value %x, expected 0x00ffffff\n", mask);
>> +
>> +  apiReturn = pConvertLengthToIpv4Mask(31, &mask);
>> +  ok(apiReturn == NO_ERROR,
>> +   "ConvertLengthToIpv4Mask returned %d, expected NO_ERROR\n", apiReturn);
>> +  ok(mask == htonl(0xfffffffe),
>> +   "ConvertLengthToIpv4Mask Mask value %x, expected 0xfeffffff\n", mask);
>> +
>> +  apiReturn = pConvertLengthToIpv4Mask(32, &mask);
>> +  ok(apiReturn == NO_ERROR,
>> +   "ConvertLengthToIpv4Mask returned %d, expected NO_ERROR\n", apiReturn);
>> +  ok(mask == htonl(0xffffffff),
>> +   "ConvertLengthToIpv4Mask Mask value %x, expected 0xffffffff\n", mask);
>> +
>> +  apiReturn = pConvertLengthToIpv4Mask(33, &mask);
>> +  ok(apiReturn ==  ERROR_INVALID_PARAMETER,
>> +   "ConvertLengthToIpv4Mask(32, &Mask) returned %d, expected ERROR_INVALID_PARAMETER\n",
>> +   apiReturn);
>> +  ok(mask == INADDR_NONE,
>> +   "ConvertLengthToIpv4Mask Mask value %x, expected  INADDR_NONE\n", mask);
> This last test would be more convincing if mask were initialized to
> 0xdeadbeef (or something).  The result of the previous test sets
> it to 0xffffffff which is INADDR_NONE.
>
> However, it probably would be better to rewrite this sequence of tests
> as a loop (from 0 to 33) calculating the expected mask in each case.
>
> Huw.

I changed the test to loop through more values. Was not able to fit the 
mask_len=33 into the loop without messing it up with too many if 
statements.

Does this look ok?

static void test_ConvertLengthToIpv4Mask(void)
{
   DWORD apiReturn;
   DWORD n;
   ULONG mask;
   ULONG expected;

   if (!pConvertLengthToIpv4Mask)
   {
     win_skip( "ConvertLengthToIpv4Mask not available\n" );
     return;
   }

   expected = 0;
   for(n=0; n <= 32; n++)
   {
     mask = 0xdeadbeef;
     if(n>0)
       expected = htonl(~0u << (32 - n));

     apiReturn = pConvertLengthToIpv4Mask(n, &mask);
     ok(apiReturn == NO_ERROR, "ConvertLengthToIpv4Mask returned 0x%.8x, 
expected 0x%.8x\n", apiReturn, NO_ERROR);
     ok(mask == expected, "ConvertLengthToIpv4Mask mask value 0x%.8x, 
expected 0x%.8x\n", mask, expected);
   }

   /* Testing for out of range. In this case both mask and return are 
changed to indicate error. */
   mask = 0xdeadbeef;
   apiReturn = pConvertLengthToIpv4Mask(33, &mask);
   ok(apiReturn ==  ERROR_INVALID_PARAMETER, "ConvertLengthToIpv4Mask 
returned 0x%.8x, expected 0x%.8x\n", apiReturn, ERROR_INVALID_PARAMETER);
   ok(mask == INADDR_NONE, "ConvertLengthToIpv4Mask mask value 0x%.8x, 
expected 0x%.8x\n", mask, INADDR_NONE);
}