gh-106844: Fix null-bytes handling in LCMapStringEx in _winapi#107832
Uh oh!
There was an error while loading. Please reload this page.
Conversation
serhiy-storchaka added needs backport to 3.11 
zooba left a comment
zooba left a comment There was a problem hiding this comment.
LGTM. Just be aware it won't backport cleanly to 3.11, and the LPCWSTR converter has a memory leak in that version and shouldn't be used.
serhiy-storchaka commented Aug 10, 2023
Before merging it I want to manually test it on Windows with large strings, and maybe add bigmem tests if it is feasible. |
| NULL, NULL, 0); | ||
| if (dest_size == 0){ | ||
| return PyErr_SetFromWindowsErr(0); | ||
| if (dest_size <= 0){ |
There was a problem hiding this comment.
It can return negative value. At least if src_size is negative. But the second call returns 0 anyway.
| if (dest_size == 0){ | ||
| return PyErr_SetFromWindowsErr(0); | ||
| if (dest_size <= 0){ | ||
| DWORD error = GetLastError(); |
There was a problem hiding this comment.
Other code calls GetLastError() as early as possible, before PyMem_Free(). Perhaps there is a reason of it.
| PyMem_Free(src_); | ||
| PyObject *ret = PyUnicode_FromWideChar(dest, nmapped); |
There was a problem hiding this comment.
It reduces the maximal total memory consumption if free this memory before allocating new memory in PyUnicode_FromWideChar().
miss-islington commented Aug 11, 2023
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12. |
miss-islington commented Aug 11, 2023
Sorry, @serhiy-storchaka, I could not cleanly backport this to |
miss-islington commented Aug 11, 2023
Sorry, @serhiy-storchaka, I could not cleanly backport this to |
…-107832) * Strings with length from 2**31-1 to 2**32-2 always caused MemoryError, it doesn't matter how much memory is available. * Strings with length exactly 2**32-1 caused OSError. * Strings longer than 2**32-1 characters were truncated due to integer overflow bug. * Strings containing the null character were truncated at the first null character. Now strings longer than 2**31-1 characters caused OverflowError and the null character is allowed.. (cherry picked from commit 04cc014) Co-authored-by: Serhiy Storchaka <[email protected]>
bedevere-bot commented Aug 11, 2023
GH-107874 is a backport of this pull request to the 3.12 branch. |
…-107832) * Strings with length from 2**31-1 to 2**32-2 always caused MemoryError, it doesn't matter how much memory is available. * Strings with length exactly 2**32-1 caused OSError. * Strings longer than 2**32-1 characters were truncated due to integer overflow bug. * Strings containing the null character were truncated at the first null character. Now strings longer than 2**31-1 characters caused OverflowError and the null character is allowed.. (cherry picked from commit 04cc014) Co-authored-by: Serhiy Storchaka <[email protected]>
bedevere-bot commented Aug 11, 2023
GH-107875 is a backport of this pull request to the 3.11 branch. |
sobolevn commented Aug 11, 2023
Thanks for fixing this! 👍 |
…-107875) * Strings with length from 2**31-1 to 2**32-2 always caused MemoryError, it doesn't matter how much memory is available. * Strings with length exactly 2**32-1 caused OSError. * Strings longer than 2**32-1 characters were truncated due to integer overflow bug. Now strings longer than 2**31-1 characters caused OverflowError. (cherry picked from commit 04cc014)
* Strings with length from 2**31-1 to 2**32-2 always caused MemoryError, it doesn't matter how much memory is available. * Strings with length exactly 2**32-1 caused OSError. * Strings longer than 2**32-1 characters were truncated due to integer overflow bug. * Strings containing the null character were truncated at the first null character. Now strings longer than 2**31-1 characters caused OverflowError and the null character is allowed.
…07874) * Strings with length from 2**31-1 to 2**32-2 always caused MemoryError, it doesn't matter how much memory is available. * Strings with length exactly 2**32-1 caused OSError. * Strings longer than 2**32-1 characters were truncated due to integer overflow bug. * Strings containing the null character were truncated at the first null character. Now strings longer than 2**31-1 characters caused OverflowError and the null character is allowed.. (cherry picked from commit 04cc014)
5 participants
Based on #106857 with some fixes.