Skip to content

Conversation

aitap
Copy link
Contributor

@aitap aitap commented Jan 7, 2025

Hello again,

Here's the pull request I've been meaning to submit back in 2020. This makes use of VirtualAlloc on WIN32 to allocate memory dynamically instead of relying on INTERNAL_MEMORY_SPACE. After ./configure --host=x86_64-w64-mingw32 I can rename and manually run the dmalloc_fc_t and dmalloc_t executables with the arguments specified for the light and heavy Makefile targets; the tests pass.

@j256
Copy link
Owner

j256 commented Jan 7, 2025

Thanks for this! I'm pretty ignorant of the Windows side of the fence. Does it make more sense to check for the include file and check for existance of the VirtualAlloc() method as opposed to just testing for WIN32?

@aitap
Copy link
Contributor Author

aitap commented Jan 8, 2025

I can't say no with 100% confidence.

People have been discussing configure tests for WinAPI in the past, but most projects seem to rely on preprocessor symbols pre-defined by the compiler (e.g. SQLite tests for _WIN32_WINNT). _WIN32 is among the Microsoft-predefined macros that everyone tests for.

On the other hand, the ancient documentation says this function is to be found in windows.h, while the current documentation (Windows ≥ XP, 2003) says "include windows.h, memoryapi.h". (And the current code in the pull request only includes memoryapi.h.)

Judging by old StackOverflow questions, older Windows SDKs may indeed lack memoryapi.h. Currently, windows.h includes memoryapi.h through winbase.h, but that doesn't seem to be documented.

I'll search the documentation some more and implement a configure test choosing between windows.h / windows.h+memoryapi.h if I don't find anything more definitive.

@j256
Copy link
Owner

j256 commented Jan 8, 2025

Thanks. I appreciate the work. Yeah I'd rather test for existence of the function and the includes as opposed to a compiler symbol.

Since the Microsoft documentation is not 100% clear on this, check for
VirtualAlloc() and VirtualFree() in either <windows.h> or <windows.h> +
<memoryapi.h>, even though the documentation may have meant "use either
header file".

Do not regenerate ./configure yet.
Pass both dmalloc_fc_t and dmalloc_t tests.
@aitap aitap force-pushed the win32-virtualalloc branch from b29b316 to 4e25a14 Compare February 13, 2025 14:45
@aitap
Copy link
Contributor Author

aitap commented Feb 13, 2025

Here are the configure tests. I made heap.c include conf.h. Would you like me to figure out a way to give it the flags via the compiler command line? Any other fixes it needs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants