<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Jonathan,<br>
<br>
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.<br>
<br>
On 12.09.2017 10:01, Jonathan Doron wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20170912080110.3051-3-jond@wizery.com">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-unicode">
<pre wrap="">Signed-off-by: Jonathan Doron <a class="moz-txt-link-rfc2396E" href="mailto:jond@wizery.com" moz-do-not-send="true"><jond@wizery.com></a>
---
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);</pre>
</div>
</blockquote>
<br>
Please use HeapAlloc().<br>
<br>
<blockquote type="cite"
cite="mid:20170912080110.3051-3-jond@wizery.com">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-unicode">
<pre wrap="">
+ 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;
+}</pre>
</div>
</blockquote>
<br>
Silent failures are not really good for tests. Please test
NtQuerySecurityObject result with ok(). Same for other similar
cases.<br>
<br>
<blockquote type="cite"
cite="mid:20170912080110.3051-3-jond@wizery.com">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-unicode">
<pre wrap="">
+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);
+}</pre>
</div>
</blockquote>
<br>
This is not really needed. You may just connect to file after it's
created, skipping new thread and ConnectNamedPipe() call.<br>
<br>
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).<br>
<br>
Thanks,<br>
Jacek<br>
</body>
</html>