Hi Andrew,
and thanks for your helpful comments! I'm attaching an accordingly updated version of the patch.
Am 2014-08-12 um 18:47 schrieb Andrew Eikum:
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?
See my answer [1] to Thomas Faber's message [2]:
Am 2014-08-12 um 19:34 schrieb Bernhard Reiter:
Including dbghelp.h along with imagehlp.h unfortunately gives a host of definition collisions. From MSDN I know that some of dbghelp's functions are mirrored in imagehlp, but apparently something's wrong here in Wine...
Am 2014-08-12 um 18:47 schrieb Andrew Eikum:
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.
Okay, got me convinced.
- (*StatusRoutine)( BindImportModule, ImageName, dll_name, 0, 0);
Does Windows really crash when given NULL StatusRoutine?
You're right, it doesn't, as an additional test [2] reveals. Added logic to catch that.
- 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.
Added logic to catch and bail out.
I would avoid whitespace changes on lines you're not otherwise changing.
That's the one thing I'd like to keep, as they're always following lines that changed (where todo_wine is removed) (modulo macros).
Bernhard
[1] https://www.winehq.org/pipermail/wine-devel/2014-August/104998.html [2] https://www.winehq.org/pipermail/wine-devel/2014-August/104996.html [3] http://source.winehq.org/patches/data/106065