[PATCH 0/1] MR2197: rpcrt4: Use the _recalloc function instead of reimplementing it.
From: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/rpcrt4/ndr_fullpointer.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/dlls/rpcrt4/ndr_fullpointer.c b/dlls/rpcrt4/ndr_fullpointer.c index 39e1b18c828..89a692324ed 100644 --- a/dlls/rpcrt4/ndr_fullpointer.c +++ b/dlls/rpcrt4/ndr_fullpointer.c @@ -87,18 +87,14 @@ static void expand_pointer_table_if_necessary(PFULL_PTR_XLAT_TABLES pXlatTables, if (RefId >= pXlatTables->RefIdToPointer.NumberOfEntries) { pXlatTables->RefIdToPointer.XlatTable = - realloc(pXlatTables->RefIdToPointer.XlatTable, sizeof(void *) * RefId * 2); + _recalloc(pXlatTables->RefIdToPointer.XlatTable, RefId * 2, sizeof(void *)); pXlatTables->RefIdToPointer.StateTable = - realloc(pXlatTables->RefIdToPointer.StateTable, RefId * 2); + _recalloc(pXlatTables->RefIdToPointer.StateTable, RefId * 2, sizeof(unsigned char)); if (!pXlatTables->RefIdToPointer.XlatTable || !pXlatTables->RefIdToPointer.StateTable) { pXlatTables->RefIdToPointer.NumberOfEntries = 0; return; } - memset(pXlatTables->RefIdToPointer.XlatTable + pXlatTables->RefIdToPointer.NumberOfEntries, 0, - (RefId * 2 - pXlatTables->RefIdToPointer.NumberOfEntries) * sizeof(void *)); - memset(pXlatTables->RefIdToPointer.StateTable + pXlatTables->RefIdToPointer.NumberOfEntries, 0, - RefId * 2 - pXlatTables->RefIdToPointer.NumberOfEntries); pXlatTables->RefIdToPointer.NumberOfEntries = RefId * 2; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2197
I don't see this function in msvcrt.dll on Windows, and I don't think we want to depend on linking to ucrtbase specifically. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2197#note_24303
On Wed Feb 15 08:39:54 2023 +0000, Nikolay Sivov wrote:
I don't see this function in msvcrt.dll on Windows, and I don't think we want to depend on linking to ucrtbase specifically. Yeah, I'm not convinced this is worth the effort.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2197#note_24305
On Wed Feb 15 08:39:54 2023 +0000, Huw Davies wrote:
Yeah, I'm not convinced this is worth the effort. My goal here was to take code that was made ugly by using CRT allocation functions and restore it to its original simplicity. When I did the CRT rewrite, I didn't know about _recalloc, and now I'm trying to undo the damage caused by my ignorance.
What would the advantage be to dropping the dependency on ucrtbase? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2197#note_24453
On Wed Feb 15 21:39:57 2023 +0000, Alex Henrie wrote:
My goal here was to take code that was made ugly by using CRT allocation functions and restore it to its original simplicity. When I did the CRT rewrite, I didn't know about _recalloc, and now I'm trying to undo the damage caused by my ignorance. What would the advantage be to dropping the dependency on ucrtbase? The advantage is to match what Windows does. We already use msvcrt for advapi32, see https://bugs.winehq.org/show_bug.cgi?id=51465.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2197#note_24524
On Thu Feb 16 11:06:55 2023 +0000, Nikolay Sivov wrote:
The advantage is to match what Windows does. We already use msvcrt for advapi32, see https://bugs.winehq.org/show_bug.cgi?id=51465. Interesting, thank you for pointing me to that bug report. Knowing that multiple applications expect msvcrt to be loaded automatically when loading a system DLL, why does Wine use ucrtbase for system DLLs at all? More importantly, how can I know which of the two I should use when implementing Wine DLLs? I found commit 05b774fb48dac5f412e3490313c6e70199013568, but there was no explanation in the commit message. @jacek?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2197#note_24616
On Fri Feb 17 06:31:23 2023 +0000, Alex Henrie wrote:
Interesting, thank you for pointing me to that bug report. Knowing that multiple applications expect msvcrt to be loaded automatically when loading a system DLL, why does Wine use ucrtbase for system DLLs at all? More importantly, how can I know which of the two I should use when implementing Wine DLLs? I found commit 05b774fb48dac5f412e3490313c6e70199013568, but there was no explanation in the commit message. @jacek? I don't remember why it was done like that, maybe for testing purposes, maybe there is some connection to mingw-w64. Tests are built with msvcrt, so that we can run them on any clean Windows installation. You don't have to know which one to use, as long as you use common subset that both crts provide.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2197#note_24618
On Fri Feb 17 06:41:18 2023 +0000, Nikolay Sivov wrote:
I don't remember why it was done like that, maybe for testing purposes, maybe there is some connection to mingw-w64. Tests are built with msvcrt, so that we can run them on any clean Windows installation. You don't have to know which one to use, as long as you use common subset that both crts provide. Never mind, I just found Jacek's explanation at <https://bugs.winehq.org/show_bug.cgi?id=51465#c28>:
It's not clear to me that linking to msvcrt.dll would make things much better. We would then be possibly missing ucrtbase.dll being loaded. On Windows, ucrtbase.dll ends up being used in quite some cases and I think that usually both msvcrt.dll and ucrtbase.dll end up being loaded.
For each DLL where _recalloc would be nice, I checked its dependencies on Windows 11 with Dependency Walker. All of them had msvcrt as a dependency and not ucrtbase. Even user32 had msvcrt as a dependency. So, it seems to me that ucrtbase being loaded instead of or in addition to msvcrt is the exception rather than the rule, and it would be better to make msvcrt the default in Wine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2197#note_24619
This merge request was closed by Alex Henrie. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2197
On Fri Feb 17 06:41:51 2023 +0000, Alex Henrie wrote:
Never mind, I just found Jacek's explanation at <https://bugs.winehq.org/show_bug.cgi?id=51465#c28>:
It's not clear to me that linking to msvcrt.dll would make things much better. We would then be possibly missing ucrtbase.dll being loaded. On Windows, ucrtbase.dll ends up being used in quite some cases and I think that usually both msvcrt.dll and ucrtbase.dll end up being loaded. For each DLL where _recalloc would be nice, I checked its dependencies on Windows 11 with Dependency Walker. All of them had msvcrt as a dependency and not ucrtbase. Even user32 had msvcrt as a dependency. So, it seems to me that ucrtbase being loaded instead of or in addition to msvcrt is the exception rather than the rule, and it would be better to make msvcrt the default in Wine. Really though, the existence of even one Windows DLL that loads both CRT libraries means that Wine is going to need a mechanism to do the same. So there may not be much point in arguing about which library is a better default when we're going to want both.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2197#note_24629
participants (4)
-
Alex Henrie -
Alex Henrie (＠alexhenrie)
-
Huw Davies (＠huw)
-
Nikolay Sivov (＠nsivov)