move curlx_ functions into lib/curlx/ #17253
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
bagder
added a commit
th
8000
at referenced
this pull request
May 6, 2025
The first step to move all curlx_ function into its own subdir. The idea is to use the curlx_ prefix proper on these functions, and use these same function names both in tool, lib and test suite source code. Stop the previous special #define setup for curlx_ names. The printf defines are now done for the library alone. Tests no longer use the printf defines. The tool code sets its own defines. The printf functions are not curlx, they are publicly available. The strcase defines are not curlx_ functions and should not be used by tool or server code. dynbuf, warnless, base64 are now proper curlx. Closes #17253
|
I will try it but just from looking at the changes I think one thing that could be an issue is the include needs the same transformation as vtls because the directory isn't included. Like there's edit: ah since curlx is used by both tool and lib then maybe it would be better to add it to the list of AdditionalIncludeDirectories in the project templates rather than change the #include? edit2: ok I see now that's what you did in src/Makefile.am so I'll do something similar to AdditionalIncludeDirectories for building the curl tool. |
|
The two fixups when combined with #17263 it works, although the CI won't show it because it won't combine them. I tested just now in Visual Studio 2010 after cherry picking from the other pr |
|
thanks a lot for your help @jay |
bagder
added a commit
that referenced
this pull request
May 7, 2025
Move curlx_ functions into its own subdir. The idea is to use the curlx_ prefix proper on these functions, and use these same function names both in tool, lib and test suite source code. Stop the previous special #define setup for curlx_ names. The printf defines are now done for the library alone. Tests no longer use the printf defines. The tool code sets its own defines. The printf functions are not curlx, they are publicly available. The strcase defines are not curlx_ functions and should not be used by tool or server code. dynbuf, warnless, base64 are now proper curlx. When libcurl is built statically, the functions from the library can be used as-is. The key is then that the functions must work as-is, without having to be recompiled for use in tool/tests. This avoids symbol collisions - when libcurl is built statically, we use those functions directly when building the tool/tests. When libcurl is shared, we build/link them separately for the tool/tests. Assisted-by: Jay Satiro Closes #17253
|
Because of the size of this patch, I'm now thinking I can land this as a first version to reduce merge conflict risks with whatever else is done at the same time as this. Then work on cleaning up the remaining curlx_ functions in follow-up PRs. @vszakats @icing as this is a monster patch, do you have any concerns before merge? |
bagder
added a commit
that referenced
this pull request
May 7, 2025
Move curlx_ functions into its own subdir. The idea is to use the curlx_ prefix proper on these functions, and use these same function names both in tool, lib and test suite source code. Stop the previous special #define setup for curlx_ names. The printf defines are now done for the library alone. Tests no longer use the printf defines. The tool code sets its own defines. The printf functions are not curlx, they are publicly available. The strcase defines are not curlx_ functions and should not be used by tool or server code. dynbuf, warnless, base64, strparse, timeval, timediff are now proper curlx functions. When libcurl is built statically, the functions from the library can be used as-is. The key is then that the functions must work as-is, without having to be recompiled for use in tool/tests. This avoids symbol collisions - when libcurl is built statically, we use those functions directly when building the tool/tests. When libcurl is shared, we build/link them separately for the tool/tests. Assisted-by: Jay Satiro Closes #17253
bagder
added a commit
that referenced
this pull request
May 7, 2025
Move curlx_ functions into its own subdir. The idea is to use the curlx_ prefix proper on these functions, and use these same function names both in tool, lib and test suite source code. Stop the previous special #define setup for curlx_ names. The printf defines are now done for the library alone. Tests no longer use the printf defines. The tool code sets its own defines. The printf functions are not curlx, they are publicly available. The strcase defines are not curlx_ functions and should not be used by tool or server code. dynbuf, warnless, base64, strparse, timeval, timediff are now proper curlx functions. When libcurl is built statically, the functions from the library can be used as-is. The key is then that the functions must work as-is, without having to be recompiled for use in tool/tests. This avoids symbol collisions - when libcurl is built statically, we use those functions directly when building the tool/tests. When libcurl is shared, we build/link them separately for the tool/tests. Assisted-by: Jay Satiro Closes #17253
Move curlx_ functions into its own subdir. The idea is to use the curlx_ prefix proper on these functions, and use these same function names both in tool, lib and test suite source code. Stop the previous special #define setup for curlx_ names. The printf defines are now done for the library alone. Tests no longer use the printf defines. The tool code sets its own defines. The printf functions are not curlx, they are publicly available. The strcase defines are not curlx_ functions and should not be used by tool or server code. dynbuf, warnless, base64, strparse, timeval, timediff are now proper curlx functions. When libcurl is built statically, the functions from the library can be used as-is. The key is then that the functions must work as-is, without having to be recompiled for use in tool/tests. This avoids symbol collisions - when libcurl is built statically, we use those functions directly when building the tool/tests. When libcurl is shared, we build/link them separately for the tool/tests. Assisted-by: Jay Satiro Closes #17253
|
Thumbs up to merging! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discussed on curl up 2025.
And use the prefix consistently.
The printf defines are now done for the library alone. Tests no longer use the printf defines. The tool code sets its own defines. The printf functions are not curlx, they are publicly available.
The strcase defines are not curlx_ functions and should not be used by tool or server code.
When libcurl is built statically, the functions from the library can be used as-is. The key is then that the functions must work as-is, without having to be recompiled for use in tool/tests. This avoids symbol collisions - when libcurl is built statically, we use those functions directly when building the tool/tests. When libcurl is shared, we build/link them separately for the tool/tests.
Saved for later