Hi Bruno,
Bruno Jesus wrote:
On Sun, Feb 5, 2017 at 5:57 PM, Julius Schwartzenberg julius.schwartzenberg@gmail.com wrote:
dlls/secur32/tests/Makefile.in | 1 + dlls/secur32/tests/lsa.c | 215 +++++++++++++++++++++++++++++++++++++++++ include/ntsecapi.h | 38 ++++++++ 3 files changed, 254 insertions(+) create mode 100644 dlls/secur32/tests/lsa.c
Hi, thanks for the patch. Unfortunately as is it cannot be used. Somethings are stated below but mostly you have to check the compiler warnings and guarantee there is none. Also, although very minor, git apply complains about 7 lines adding trailing whitespaces.
Thanks a lot for the review! I hope to send a better version soon. I just wasn't sure whether wine-patches was already appropriate for non-final patches or not. It seems nice that Marvin runs the tests already. Let me know if I'd better send it differently before the review process is finished.
- Lsa tests
The copyright line is missing, example:
Copyright 2017 Julius Schwartzenberg
I wasn't sure whether this was important, but I can add that.
+static void test_LsaLookupAuthenticationPackage_correct_handle(void) +{
- NTSTATUS status;
- HANDLE lsa_handle;
- LSA_STRING package_name;
- ULONG auth_package;
- package_name.Buffer = package_name_kerberos;
- package_name.Length = (USHORT)strlen(package_name.Buffer);
- LsaConnectUntrusted(&lsa_handle);
- status = LsaLookupAuthenticationPackage(lsa_handle, &package_name, &auth_package);
- ok(status == STATUS_SUCCESS,
"LsaLookupAuthenticationPackage(valid_handle, \"Kerberos\", ...) returned 0x%x\n", status);Is it possible to test that auth_package somehow?
Possibly yeah. I haven't figured out much yet. I just started to write a bunch of tests some time ago to see whether I could figure things out. I mailed it now because it seemed better to submit it after the 2.0 release. Would it make sense to add this test in a second patch?
These tests so far are really really basic and it seemed to make sense to me to add more tests in later patches. (Or first solve some todo_wines?)
diff --git a/include/ntsecapi.h b/include/ntsecapi.h index 2bb3d31..439f67e 100644 --- a/include/ntsecapi.h +++ b/include/ntsecapi.h @@ -387,6 +387,44 @@ NTSTATUS WINAPI LsaSetTrustedDomainInformation(LSA_HANDLE,PSID,TRUSTED_INFORMATI NTSTATUS WINAPI LsaStorePrivateData(LSA_HANDLE,PLSA_UNICODE_STRING,PLSA_UNICODE_STRING); NTSTATUS WINAPI LsaUnregisterPolicyChangeNotification(POLICY_NOTIFICATION_INFORMATION_CLASS,HANDLE);
+#ifndef MICROSOFT_KERBEROS_NAME_A
+#define MICROSOFT_KERBEROS_NAME_A "Kerberos" +#define MICROSOFT_KERBEROS_NAME_W L"Kerberos"
You can't use L to define WCHAR data.
+#ifdef WIN32_CHICAGO
Where is this from? Not a single Wine header has that name.
I copy pasted the headers from MinGW. On IRC I was told that is ok because the file starts with "This file has no copyright assigned and is placed in the Public Domain.".
I didn't consider any technical issues though. How should MinGW/Wine header differences be dealt with?
Thanks again for the feedback! I hope I can send a better patch soon.
Best regards, Julius