Re: [PATCH] mpr: Implement connected resources enumeration.

[Hugh McMaster]

Hi Pierre,

Thanks for working on this. Some comments below.

Hugh

On Tuesday, 13 September 2016 5:53 AM, Pierre Schweitzer wrote:
> @@ -73,6 +74,7 @@ typedef struct _WNetProviderTable
> #define WNET_ENUMERATOR_TYPE_GLOBAL   1
> #define WNET_ENUMERATOR_TYPE_PROVIDER 2
> #define WNET_ENUMERATOR_TYPE_CONTEXT  3
> +#define WNET_ENUMERATOR_TYPE_CONNECTED 4

Please align all of the #defines using two or four spaces after the definition.
> - LPNETRESOURCEW lpNet;
> - union
> - {
> -
>    NETRESOURCEW * net;
> -
>    HANDLE * handles;
> - };

We prefer the * to sit directly adjacent to the variable name, so NETRESOURCEW *net (i.e. no space before the variable name).
> +static PWNetEnumerator _createConnectedEnumerator(DWORD dwScope, DWORD dwType,
> - DWORD dwUsage)
>
> +{
> - PWNetEnumerator ret = HeapAlloc(GetProcessHeap(),
> -
> HEAP_ZERO_MEMORY, sizeof(WNetEnumerator));

These lines do not need to be split.
> +#define _copyStringToEnumW(s)            \
> - len = strlenW(buffer[i].s) + 1;      \
> - len *= sizeof(WCHAR);                \
> - if (left < len)                      \
> - {                                    \
> -
>    ret = WN_MORE_DATA;              \
> -
>    break;                           \
> - }                                    \
> - end = (PVOID)((ULONG_PTR)end - len); \
> - curr->s = end;                       \
> - memcpy(end, buffer[i].s, len);       \
> - left -= len;

This would be better understood as a helper function.
> +static DWORD _enumerateConnectedW(PWNetEnumerator enumerator, DWORD * user_count,
> - VOID * user_buffer, DWORD * user_size)

Please fix the * position. Also, please align the second line of arguments beneath PWNetEnumerator.
'VOID' should be 'void'.
> +{
> - DWORD ret, index, count, size, i, len, left;
> - PVOID end;
> - LPNETRESOURCEW curr, buffer;
> - PHANDLE handles;

Avoid LPnnn and Pnnn syntax. Use void *, NETRESOURCEW * and HANDLE * instead. This mistake
happens later on in your patch too.
> - if (!user_count)
> -
>    return WN_BAD_POINTER;
> - if (!user_buffer)
> -
>    return WN_BAD_POINTER;
> - if (!user_size)
> -
>    return WN_BAD_POINTER;

Unless you plan to change this code in another patch, you can combine these into two lines.
> -
>        if (handles[index] == 0)
> -
>        {
> -
>            ret = providerTable->table[index].openEnum(enumerator->dwScope,
> -
>                                                       enumerator->dwType,
> -
>                                                       enumerator->dwUsage,
> -
>                                                       NULL, &handles[index]);
> -
>            if (ret != WN_SUCCESS)
> -
>                continue;
> -
>        }
> -
> -
>        ret = providerTable->table[index].enumResource(handles[index],
> -
>                                                       &count,
> -
>                                                       buffer,
> -
>                                                       &size);

Line formatting.
> -
>            for (index = 0; index < providerTable->numProviders; index++)
> -
>            {
> -
>                if (providerTable->table[index].dwEnumScopes && handles[index] != 0)
> -
>                    providerTable->table[index].closeEnum(handles[index]);
> -
>            }

You can remove the != 0 statement since you don't use it for providerTable->table[index].dwEnumScopes.