-
-
Notifications
You must be signed in to change notification settings - Fork 7k
curlx: add curlx_rename(), fix to support long filenames on Windows
#20042
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
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Curl_rename to curlx_rename [WIP]Curl_rename to curlx_rename, add long filename support on Windows
Curl_rename to curlx_rename, add long filename support on Windowscurlx_rename, with long filename support on Windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the file rename functionality by introducing curlx_rename with long filename support on Windows, replacing the previous Curl_rename() function. The key change is moving the implementation from standalone files to the existing curlx/fopen module and aligning the error return value with POSIX conventions (-1 for failure instead of 1).
- Adds
curlx_win32_rename()with Windows long path support usingfix_excessive_path() - Updates three call sites (hsts.c, cookie.c, altsvc.c) to use the new
curlx_renameAPI - Adds
renameandMoveFileEx*functions to the banned functions list to enforce use of the wrapper
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/checksrc.pl | Adds MoveFileEx, MoveFileExA, MoveFileExW, and rename to banned functions list |
| lib/rename.h | Removes old header file containing Curl_rename declaration |
| lib/rename.c | Removes old implementation file |
| lib/hsts.c | Updates call from Curl_rename() to curlx_rename() and removes now-unnecessary include |
| lib/curlx/fopen.h | Adds curlx_win32_rename declaration and defines curlx_rename macro |
| lib/curlx/fopen.c | Implements curlx_win32_rename() with long filename support via fix_excessive_path() |
| lib/cookie.c | Updates call from Curl_rename() to curlx_rename() and removes include |
| lib/altsvc.c | Updates call from Curl_rename() to curlx_rename() and removes include |
| lib/Makefile.inc | Removes rename.c and rename.h from build files list |
| docs/internals/CODE_STYLE.md | Documents rename and MoveFileEx* as banned functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
curlx_rename, with long filename support on Windowscurlx_rename(), with long filename support on Windows
Same would need to apply to the fix_excessive_path worker function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
curlx_rename(), with long filename support on Windowscurlx_rename(), fix long filename support on Windows
curlx_rename(), fix long filename support on Windowscurlx_rename(), fix to support long filenames on Windows
|
Yes when UNICODE is defined we expect _UNICODE and vice versa that's why there's a compile-time sanity check in setup-win32 |
Move existing
Curl_rename()rename()wrapper from lib tocurlx/fopen, and make it a curlx macro/function. To allow using
the local worker function to fixup long filenames on Windows.
Then fix the Windows-specific rename implementation to support long
filenames. This operation may happen when using a cookie jar, HSTS cache
or alt-svc cache, via libcurl or the curl tool.
Before this patch, when passing a long filename to the above options,
a
<random>.tmpfile was left on the disk without renaming it to thefilename passed to curl. There was also 1 second delay for each
attempted rename operation.
Also:
rename()andMoveFileEx*()functions.Curl_rename()returned 1 on failure before this patch, whilecurlx_rename()returns -1 after, to match POSIXrename().Refs:
https://learn.microsoft.com/windows/win32/api/winbase/nf-winbase-movefileexa
https://learn.microsoft.com/windows/win32/fileio/maximum-file-path-limitation
Ref: #20040
CreateFile()calls to support long filenames #20040.curlx_win32_rename().UNICODE!=_UNICODE.Though I believe such build makes little practical sense, let alone supporting it.
setup-win32.hhas protection against mixing them, soUNICODE == _UNICODEis guaranteed. Nice!