curl_multibyte: always return a heap-allocated copy of string #6602
Conversation
MarcelRaad
left a comment
MarcelRaad
left a comment
There was a problem hiding this comment.
Doesn't build yet, but I like the usage simplification. Can't curlx_unicodefree just be removed completely?
|
👉 View analysis in DeepCode’s Dashboard | Configure the bot |
Ok I replaced the calls with Curl_safefree. I don't understand why |
There was some CI failure after using the macro for non-Windows too, but I don't remember the details. |
jay
force-pushed
the
improve_multibyte
branch
from
38071e6 to
dda90a0
Compare
So it appears I can't get rid of curlx_unicodefree. Since the conversion functions are curlx they are not tracked by memdebug which is why that (free) was in parentheses. I've reverted the change to Curl_safefree and added comments explaining why the function-like macros in multibyte use parentheses. |
jay
force-pushed
the
improve_multibyte
branch
from
dda90a0 to
e59dea7
Compare
- Change the Windows char <-> UTF-8 conversion functions to return an allocated copy of the passed in string instead of the original. Prior to this change the curlx_convert_ functions would, as what I assume was an optimization, not make a copy of the passed in string if no conversion was required. No conversion is required in non-UNICODE Windows builds since our tchar strings are type char and remain in whatever the passed in encoding is, which is assumed to be UTF-8 but may be other encoding. In contrast the UNICODE Windows builds require conversion (wchar <-> char) and do return a copy. That inconsistency could lead to programming errors where the developer expects a copy, and does not realize that won't happen in all cases. Closes #xxxx
jay
force-pushed
the
improve_multibyte
branch
from
e59dea7 to
ce0e186
Compare
|
It seems this change causes the NTLM authentication to crash. Crashes happen in Curl_auth_build_spn -> _CrtIsValidHeapPointer. |
Please try #6938 |
curlx_convert_UTF8_to_tchar must be freed by curlx_unicodefree, but prior to this change some uses mistakenly called free. I've reviewed all other uses of curlx_convert_UTF8_to_tchar and curlx_convert_tchar_to_UTF8. Bug: curl#6602 (comment) Reported-by: [email protected] Closes #xxxx
|
#6938 fixes the issue I have commented. Thank you! |
curlx_convert_UTF8_to_tchar must be freed by curlx_unicodefree, but prior to this change some uses mistakenly called free. I've reviewed all other uses of curlx_convert_UTF8_to_tchar and curlx_convert_tchar_to_UTF8. Bug: #6602 (comment) Reported-by: [email protected] Closes #6938
3 participants
allocated copy of the passed in string instead of the original.
Prior to this change the curlx_convert_ functions would, as what I
assume was an optimization, not make a copy of the passed in string if
no conversion was required. No conversion is required in non-UNICODE
Windows builds since our tchar strings are type char and already in
UTF-8, so no conversion takes place.
In contrast the UNICODE Windows builds require conversion
(wchar <-> char) and do return a copy. That inconsistency could lead to
programming errors where the developer expects a copy, and does not
realize that won't happen in all cases.
Closes #xxxx