Re: [1/2] imagehlp: Implement (parts of) BindImageEx (try 10)

[Jacek Caban]

Hi Bernhard,

On 06/30/14 19:15, Bernhard Reiter wrote:
> diff --git a/dlls/imagehlp/tests/image.c b/dlls/imagehlp/tests/image.c
> index e008b0b..d93130c 100644
> --- a/dlls/imagehlp/tests/image.c
> +++ b/dlls/imagehlp/tests/image.c
> @@ -31,6 +31,9 @@
>  static HMODULE hImageHlp;
>
> static BOOL (WINAPI *pImageGetDigestStream)(HANDLE, DWORD, DIGEST_FUNCTION, DIGEST_HANDLE);
> +static BOOL (WINAPI *pBindImageEx)(DWORD Flags, PCSTR ImageName,
> -
>                               PCSTR DllPath, PCSTR SymbolPath,
> -
>                               PIMAGEHLP_STATUS_ROUTINE StatusRoutine);
>
> /* minimal PE file image */
>  #define VA_START 0x400000
> @@ -149,6 +152,9 @@ struct expected_update_accum
>      BOOL  todo;
>  };
>
> +static int status_routine_called[14] = {0};
> -
> -
>
> static BOOL WINAPI accumulating_stream_output(DIGEST_HANDLE handle, BYTE *pb,
>   DWORD cb)
>  {
> @@ -273,6 +279,52 @@ static void update_checksum(void)
>      bin.nt_headers.OptionalHeader.CheckSum = sum;
>  }
>
> +BOOL WINAPI TestingStatusRoutine(IMAGEHLP_STATUS_REASON reason, PSTR ImageName, PSTR DllName,
> -
>                             ULONG_PTR Va, ULONG_PTR Parameter)
>
> +{

Make it static.
> - char kernel32_path[MAX_PATH];
> -
> - status_routine_called[reason]++;
> -
> - todo_wine ok((reason == BindImportModule) || (reason == BindImportProcedure) ||
> -
>             (reason == BindForwarderNOT), "expected reason to be one of BindImportModule, "
> -
>             "BindImportProcedure, or BindForwarderNOT, got %d\n", reason);

How about moving that to the switch that you need anyway?
> -
> - switch(reason)
> - {
> -
>    case BindOutOfMemory:
> -
>    case BindRvaToVaFailed:
> -
>    case BindNoRoomInImage:
> -
>    case BindImportModuleFailed:
> -
>    case BindImportProcedureFailed:
> -
>        break;

How about

default: ok(0, "...");
> -
> -
>    case BindImportModule:
> -
>        ok(!strcmp(DllName, "KERNEL32.DLL"), "expected DllName to be KERNEL32.DLL, got %s\n",
> -
>           DllName);
> -
>        break;
> -
> -
>    case BindImportProcedure:
> -
>        GetSystemDirectoryA(kernel32_path, MAX_PATH);
> -
>        strcat(kernel32_path, "\\KERNEL32.DLL");
> -
>        ok(!lstrcmpiA(DllName, kernel32_path), "expected DllName to be %s, got %s\n",
> -
>           kernel32_path, DllName);
> -
>        ok(!strcmp((LPSTR)Parameter, "ExitProcess"),
> -
>           "expected Parameter to be ExitProcess, got %s\n", (LPSTR)Parameter);
> -
>        break;
> -
> -
>    case BindForwarder:
> -
>    case BindForwarderNOT:
> -
>    case BindImageModified:
> -
>    case BindExpandFileHeaders:
> -
>    case BindImageComplete:
> -
>    case BindMismatchedSymbols:
> -
>    case BindSymbolsNotUpdated:
> -
>        break;
> - }

Same here. You really need cases for expected values.
> - return TRUE;
>
> +}
> -
>
> static void test_get_digest_stream(void)
>  {
>      BOOL ret;
> @@ -329,6 +381,54 @@ static void test_get_digest_stream(void)
>      DeleteFileA(temp_file);
>  }
>
> +static void test_bind_image_ex(void)
> +{
> - BOOL ret;
> - HANDLE file;
> - char temp_file[MAX_PATH];
> - DWORD count;
> -
> - /* Call with a non-existant file */
> - SetLastError(0xdeadbeef);
> - ret = pBindImageEx(BIND_NO_BOUND_IMPORTS | BIND_NO_UPDATE | BIND_ALL_IMAGES, temp_file, NULL,
> -
>                   NULL, (void *)TestingStatusRoutine);
> - todo_wine ok(!ret && ((GetLastError() == ERROR_FILE_NOT_FOUND) ||
> -
>             (GetLastError() == ERROR_INVALID_PARAMETER)),
> -
>             "expected ERROR_FILE_NOT_FOUND or ERROR_INVALID_PARAMETER, got %d\n",
> -
>             GetLastError());
> -
> - file = create_temp_file(temp_file);
> - if (file == INVALID_HANDLE_VALUE)
> - {
> -
>    skip("couldn't create temp file\n");
> -
>    return;
> - }
> -
> - WriteFile(file, &bin, sizeof(bin), &count, NULL);
> - FlushFileBuffers(file);
> - CloseHandle(file);
> -
> - /* Finally, call with a proper PE file */
> - SetLastError(0xdeadbeef);
> - ret = pBindImageEx(BIND_NO_BOUND_IMPORTS | BIND_NO_UPDATE | BIND_ALL_IMAGES, temp_file, NULL,
> -
>                   NULL, (void *)TestingStatusRoutine);
> - ok(ret, "BindImageEx failed: %d\n", GetLastError());
> -
> - todo_wine ok(status_routine_called[BindImportModule] == 1,
> -
>             "Expected StatusRoutine to be called once with reason BindImportModule, "
> -
>             "but it was called %d times\n", status_routine_called[BindImportModule]);
> -
> - /* Under wvistau64 (64 bit image), w2008s64 (64 bit image) and w7pro64 (64 bit image),
> -
> * StatusRoutine is called with reason BindForwarderNOT instead of BindImportProcedure. */

Please avoid such comments. Testbot VM names are not related to the code
nor permanent.

Cheers,
Jacek