[PATCH] mpr: Implement connected resources enumeration.
Hugh McMaster
hugh.mcmaster at outlook.com
Tue Sep 13 00:41:54 CDT 2016
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.
More information about the wine-devel
mailing list