-
-
Notifications
You must be signed in to change notification settings - Fork 7k
Windows: Add long path support for CreateFile operations #19286
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
|
Please fix for failing CI due to trailing whitespace in the blank lines. Also, I'm pretty sure curlx functions are not supposed to return memory-tracked memory pointers unless the comments here are wrong. So what that means is you don't return strdup(xyz) instead you have to return (strdup)(xyz) so that curlx_unicodefree can properly free it (that function just calls (free) like (free)(path)). If anyone else wants to weigh in on this , I'm not positive. |
| #ifdef _UNICODE | ||
| wchar_t *path_w = curlx_convert_UTF8_to_wchar(path); | ||
| if(path_w) { | ||
| if(fix_excessive_path(path_w, &fixed)) | ||
| target = fixed; | ||
| else | ||
| target = path_w; | ||
|
|
||
| if(target) { | ||
| result = _wcsdup(target); | ||
| (free)(fixed); | ||
| } | ||
| curlx_unicodefree(path_w); | ||
| } | ||
| #else | ||
| if(fix_excessive_path(path, &fixed)) | ||
| target = fixed; | ||
| else | ||
| target = path; | ||
|
|
||
| if(target) | ||
| result = strdup(target); | ||
|
|
||
| (free)(fixed); | ||
| #endif |
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.
On further inspection it seems you may be able to eliminate some dup. The only thing that would need strdup is the passed in path. The other pointers already point to memory that is created outside the memory tracker (in curlx_ prefixed functions or helper functions)
target = NULL;
#ifdef _UNICODE
wchar_t *path_w = curlx_convert_UTF8_to_wchar(path);
if(path_w) {
if(fix_excessive_path(path_w, &fixed)) {
target = fixed;
(free)(path_w);
}
else
target = path_w;
}
#else
if(fix_excessive_path(path, &fixed))
target = fixed;
else
target = (strdup)(path);
#endif
return target;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.
you could consolidate it even further by returning directly nm I see a flaw in that because you'd still have to return NULL in one casereturn fixed etc, that would get rid of the two lines
|
There is a 3rd |
| else | ||
| target = path_w; | ||
|
|
||
| result = (_wcsdup)(target); |
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.
| result = (_wcsdup)(target); | |
| result = _wcsdup(target); |
curl doesn't override this function, making it safe to call as-is.
| return NULL; | ||
|
|
||
| #ifdef _UNICODE | ||
| wchar_t *path_w = curlx_convert_UTF8_to_wchar(path); |
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.
CI failing due to this line. Fix is to create a block, or move the declaration to the top of the function.
C:/projects/curl/lib/curlx/fopen.c(334,12): error : mixing declarations and code is incompatible with standards before C99 [-Werror,-Wdeclaration-after-statement] [C:\projects\curl_bld\lib\libcurl_object.vcxproj]
|
Implemented slightly differently, also for all CreateFile calls in curl, via PR ##20040. Thanks for the PR and for raising the issue. |
Fixes: #8361
This PR extends Windows long path support (paths > 260 characters) to file operations using CreateFile API, particularly for getting and setting file timestamps in the curl command-line tool.
Implement curlx_win32_fix_long_path() to normalize, handle excessive/\?\ long Windows paths and return a heap-allocated TCHAR* (UTF-16 or multibyte as appropriate). Replace direct curlx_convert_UTF8_to_tchar() calls in tool_filetime with the new helper so filetime operations work with long paths.