Re: [v1 3/3] kernel32/tests: pipe: Added test for pipe security attributes
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(a)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
participants (1)
-
Jacek Caban