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.