[1/2] imagehlp/tests: Add test for BindImageEx (try 12)
Bernhard Reiter
ockham at raz.or.at
Mon Aug 4 14:49:22 CDT 2014
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 at 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
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-test-for-BindImageEx.patch
Type: text/x-patch
Size: 4804 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20140804/dbd0bbba/attachment.bin>
More information about the wine-devel
mailing list