imagehlp: Implement (parts of) BindImageEx

Andrew Eikum aeikum at codeweavers.com
Tue Aug 12 11:47:27 CDT 2014


I don't know this area of Wine, but some more general thoughts below.

On Tue, Aug 12, 2014 at 05:34:42PM +0200, Bernhard Reiter wrote:
> +void * WINAPI ImageDirectoryEntryToDataEx( void *base, BOOL image, unsigned short dir,
> +                                           unsigned long *size, PIMAGE_SECTION_HEADER *section );

Is there a reason for forward-declaring this instead of including
dbghelp.h?

>   */
>  BOOL WINAPI BindImageEx(
> -  DWORD Flags, PCSTR ImageName, PCSTR DllPath, PCSTR SymbolPath,
> +  DWORD Flags, const char *ImageName, const char *DllPath, const char *SymbolPath,
>    PIMAGEHLP_STATUS_ROUTINE StatusRoutine)
>  {
> -  FIXME("(%d, %s, %s, %s, %p): stub\n",
> +  LOADED_IMAGE loaded_image;
> +  const IMAGE_IMPORT_DESCRIPTOR *import_desc;
> +  unsigned long size;
> +  char dll_fullname[MAX_PATH];
> +
> +  FIXME("(%d, %s, %s, %s, %p): semi-stub\n",

I know it's inconsistent with the rest of the file, but I'd use
four-space indents for this function, since it's all new code and you
have the opportunity to fix it. Not a big deal if you'd rather keep
two spaces.

> +    (*StatusRoutine)( BindImportModule, ImageName, dll_name, 0, 0);
> +

Does Windows really crash when given NULL StatusRoutine?

> +    thunk = ImageRvaToVa( loaded_image.FileHeader, loaded_image.MappedAddress,
> +                          import_desc->OriginalFirstThunk, 0 );

I don't know these functions. Is it worth checking the return value of
ImageRvaToVa? MSDN implies it can fail.

> -    todo_wine ok(status_routine_called[BindImportModule] == 1,
> -                 "StatusRoutine was called %d times\n", status_routine_called[BindImportModule]);
> +    ok(status_routine_called[BindImportModule] == 1,
> +       "StatusRoutine was called %d times\n", status_routine_called[BindImportModule]);
>  
> -    todo_wine ok((status_routine_called[BindImportProcedure] == 1)
> +    ok((status_routine_called[BindImportProcedure] == 1)
>  #if defined(_WIN64)
> -                 || broken(status_routine_called[BindImportProcedure] == 0) /* < Win8 */
> +       || broken(status_routine_called[BindImportProcedure] == 0) /* < Win8 */
>  #endif
> -                 , "StatusRoutine was called %d times\n", status_routine_called[BindImportProcedure]);
> +       , "StatusRoutine was called %d times\n", status_routine_called[BindImportProcedure]);

I would avoid whitespace changes on lines you're not otherwise
changing.

Andrew



More information about the wine-devel mailing list