Skip to content

memory: stop overriding unused wcsdup()/_wcsdup() system functions #17840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Jul 7, 2025

Also ban them via checksrc.

The code continues to use _tcsdup().

Assisted-by: Daniel Stenberg

Closes #17840


@github-actions github-actions bot added the tests label Jul 7, 2025
@vszakats vszakats added Windows Windows-specific tidy-up and removed tests labels Jul 7, 2025
@vszakats vszakats changed the title windows: drop wcsdup() in favor of _wcsdup() windows: prefer _wcsdup() over deprecated wcsdup() Jul 7, 2025
@github-actions github-actions bot added the tests label Jul 7, 2025
@vszakats
Copy link
Member Author

vszakats commented Jul 7, 2025

Into the rabbit hole:
76e047f #7540
changed the initial value of Curl_cwcsdup from _wcsdup to Curl_wcsdup.
It did not apply the same change to global_init() a few lines below.
libtests also remains using _wcsdup.

Is this correct? Aren't all these supposed to be in sync?

@vszakats
Copy link
Member Author

vszakats commented Jul 7, 2025

Same has been reported last year under the original PR:
#7540 (comment)

@bagder
Copy link
Member

bagder commented Jul 7, 2025

It looks like _tcsdup() is the only call left used in libcurl code, in four different places.

@vszakats
Copy link
Member Author

vszakats commented Jul 7, 2025

Indeed!

Except when assigning the function to Curl_cwcsdup. Which still uses _wcsdup:

#if defined(_WIN32) && defined(UNICODE)
curl_wcsdup_callback Curl_cwcsdup = wcsdup;
#endif

curl/lib/easy.c

Lines 160 to 162 in f048546

#if defined(_WIN32) && defined(UNICODE)
Curl_cwcsdup = (curl_wcsdup_callback)_wcsdup;
#endif

As opposed to this fixed instance:

curl/lib/easy.c

Lines 132 to 134 in f048546

#if defined(_WIN32) && defined(UNICODE)
curl_wcsdup_callback Curl_cwcsdup = Curl_wcsdup;
#endif

@vszakats vszakats marked this pull request as draft July 7, 2025 10:47
@bagder
Copy link
Member

bagder commented Jul 7, 2025

Except when assigning the function to Curl_cwcsdup.

I was only referring to callers, users of this functionality.

@vszakats
Copy link
Member Author

vszakats commented Jul 7, 2025

Got it now!

So, deleted both macro redefs, since those are necessary for actual calls,
which do not exist.

Then fixed to use Curl_wcsdup() in global_init() (will move this to
separate PR), and changed the same in libtests to map to _wcsdup
instead of wcsdup.

Also ban them via `checksrc`.

The code continues to use `_tcsdup()`.

Closes curl#17840
vszakats added a commit to vszakats/curl that referenced this pull request Jul 7, 2025
@vszakats vszakats changed the title windows: prefer _wcsdup() over deprecated wcsdup() memory: stop overriding unused wcsdup()/_wcsdup() system functions Jul 7, 2025
@vszakats vszakats marked this pull request as ready for review July 7, 2025 11:27
@vszakats vszakats closed this in ef2ccf8 Jul 7, 2025
@vszakats vszakats deleted the wcsdup branch July 7, 2025 13:04
vszakats added a commit to vszakats/curl that referenced this pull request Jul 7, 2025
@vszakats vszakats added the Unicode Unicode, code page, character encoding label Jul 7, 2025
vszakats added a commit that referenced this pull request Jul 7, 2025
vszakats added a commit to vszakats/curl that referenced this pull request Jul 7, 2025
vszakats added a commit that referenced this pull request Jul 8, 2025
This callback was permanently mapped to libcurl's internal
`Curl_wcsdup()`, which always uses the customizable malloc for
allocation, thus making a custom mapping redundant anyway.

To simplify, drop the callback and map `_tcsdup()` in Unicode mode
directly to `Curl_wcsdup()`.

Also fixes:
- `curl_global_init()` which, before this patch, (re)initialized its
  mapping to `_wcsdup()`, returning buffers potentially incompatible
  with a custom allocator.
  Bug: #17840 (comment)
  Bug: #7540 (comment)
  Co-reported-by: Luca Kellermann

Follow-up to 76e047f #7540
Assisted-by: Jay Satiro

Closes #17843
vszakats added a commit to vszakats/curl that referenced this pull request Jul 25, 2025
vszakats added a commit that referenced this pull request Jul 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests tidy-up Unicode Unicode, code page, character encoding Windows Windows-specific
Development

Successfully merging this pull request may close these issues.

2 participants