[v1 3/3] kernel32/tests: pipe: Added test for pipe security attributes
Jacek Caban
jacek at codeweavers.com
Mon Sep 18 12:02:23 CDT 2017
Hi Jonathan,
As I mentioned in the other mail, it would be interesting to add some
more tests. As for those tests, I have a few comments bellow.
On 12.09.2017 10:01, Jonathan Doron wrote:
> Signed-off-by: Jonathan Doron <jond at wizery.com>
> ---
> dlls/kernel32/tests/pipe.c | 241 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 241 insertions(+)
>
> diff --git a/dlls/kernel32/tests/pipe.c b/dlls/kernel32/tests/pipe.c
> index 0a6edd6..9b90ce9 100644
> --- a/dlls/kernel32/tests/pipe.c
> +++ b/dlls/kernel32/tests/pipe.c
> @@ -28,6 +28,7 @@
> #include "winternl.h"
> #include "winioctl.h"
> #include "wine/test.h"
> +#include "sddl.h"
>
> #define PIPENAME "\\\\.\\PiPe\\tests_pipe.c"
>
> @@ -3018,6 +3019,245 @@ static void test_overlapped_transport(BOOL msg_mode, BOOL msg_read_mode)
> CloseHandle(client);
> }
>
> +static PSECURITY_DESCRIPTOR GetObjectSecDesc(HANDLE Object)
> +{
> + ULONG RequiredLength;
> + PSECURITY_DESCRIPTOR SecDesc = NULL;
> +
> + RequiredLength = 0;
> + NtQuerySecurityObject(Object,
> + GROUP_SECURITY_INFORMATION | OWNER_SECURITY_INFORMATION,
> + NULL, 0, &RequiredLength); // Expected c0000023
> + if (!RequiredLength)
> + goto cleanup;
> +
> + SecDesc = LocalAlloc(LMEM_FIXED, RequiredLength);
Please use HeapAlloc().
> + if (!SecDesc)
> + goto cleanup;
> + memset(SecDesc, 0, RequiredLength);
> +
> + if (NtQuerySecurityObject(Object,
> + GROUP_SECURITY_INFORMATION | OWNER_SECURITY_INFORMATION,
> + SecDesc, RequiredLength, &RequiredLength)) {
> + LocalFree(SecDesc);
> + SecDesc = NULL;
> + goto cleanup;
> + }
> +
> +cleanup:
> + return SecDesc;
> +}
Silent failures are not really good for tests. Please test
NtQuerySecurityObject result with ok(). Same for other similar cases.
> +static ULONG WINAPI ServerPipeThread(PVOID Param)
> +{
> + ULONG RetVal;
> +
> + RetVal = ConnectNamedPipe((HANDLE)Param, NULL) ? ERROR_SUCCESS : GetLastError();
> + if ((RetVal == ERROR_PIPE_CONNECTED) || (RetVal == ERROR_SUCCESS))
> + RetVal = ERROR_SUCCESS;
> +
> + ExitThread(RetVal);
> +}
This is not really needed. You may just connect to file after it's
created, skipping new thread and ConnectNamedPipe() call.
Also there are styling issues. Please don't use C++ comments (//) in
Wine and try to match style of existing code (eg. function names,
variable names).
Thanks,
Jacek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20170918/06b75a18/attachment.html>
More information about the wine-devel
mailing list