Okay, as there hasn't been any response, I've tried to fix these issues as well as I could on my own. Looking forward to comments! Bernhard Am 2014-07-25 um 18:22 schrieb Bernhard Reiter:
Am 2014-07-24 um 18:33 schrieb Alexandre Julliard:
Bernhard Reiter <ockham(a)raz.or.at> writes:
@@ -31,6 +31,8 @@ static HMODULE hImageHlp;
static BOOL (WINAPI *pImageGetDigestStream)(HANDLE, DWORD, DIGEST_FUNCTION, DIGEST_HANDLE); +static BOOL (WINAPI *pBindImageEx)(DWORD Flags, const char *ImageName, const char *DllPath, + const char *SymbolPath, void *StatusRoutine);
That's not the correct prototype.
Meaning I should change the const char*s to char*s (as MSDN says PSTR)? I was looking at the BindImageEx stub in dlls/imagehlp/modify.c, and that had PCSTRs.
(Or do you mean using PIMAGEHLP_STATUS_ROUTINE instead of void* for StatusRoutine? Because that would require me to typecast testing_status_routine to that when invoking pBindImageEx, wouldn't it?)
+static BOOL WINAPI testing_status_routine(IMAGEHLP_STATUS_REASON reason, char *ImageName, + char *DllName, ULONG_PTR Va, ULONG_PTR Parameter) +{ + char kernel32_path[MAX_PATH]; + + status_routine_called[reason]++;
This will have nasty results if you get an unexpected code.
Okay, I'll add checks if reason is in the expected range.
+ default: + ok(0, "expected reason to be one of BindImportModule, BindImportProcedure, or " + "BindForwarderNOT, got %d\n", reason);
Please simplify your error messages. In particular, this one would need to be changed every time the test is expanded, that's not a good idea.
I'm not exactly creative how to change this... would
ok(0, "testing_status_routine got wrong reason %d\n", reason);
be okay?
+static void test_bind_image_ex(void) +{ + BOOL ret; + HANDLE file; + char temp_file[MAX_PATH] = "nonexistant.dll";
There's no reason to use a variable for this.
But I need it later to retrieve the name of the file created by create_temp_file. Plus, if I change the const char*s in pBindImageEx's declaration to char*s, I will run into problems here if I pass a const char* to pBindImageEx, won't I?
So is it okay to leave it as it is or am I overlooking something?
+ /* Under 64 bit images of Vista Ultimate, 2008 Server, and 7 Pro, StatusRoutine is called with + * reason BindForwarderNOT instead of BindImportProcedure. */ + todo_wine ok((status_routine_called[BindImportProcedure] == 1) || + (status_routine_called[BindForwarderNOT] == 1), + "Expected StatusRoutine to be called once with reason BindImportProcedure or " + "BindForwarderNOT, but it was called %d and %d times, respectively\n", + status_routine_called[BindImportProcedure], status_routine_called[BindForwarderNOT]);
You'd most likely want the test to depend on 32/64-bit instead of always accepting both.
I had #if defined(_WIN64) macros there earlier, but the behavior isn't consistent: Windows 8 gives BindImportProcedure also with the 64 bit image. (I tried it with test job 7953, if that can be retrieved somehow. Could that BindForwarderNOT be due to a bug in WoW that was fixed in Windows 8?) I asked on #winehackers if I should add a check for the Windows version, but people suggested not to. What's your stance on this?
Bernhard