Skip to content

Michal/erts/fix-handling-of-short-paths-on-windows/OTP-19690 #9996

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

Mikaka27
Copy link
Contributor

@Mikaka27 Mikaka27 commented Jun 25, 2025

No description provided.

Copy link
Contributor

github-actions bot commented Jun 25, 2025

CT Test Results

    3 files    142 suites   49m 34s ⏱️
1 649 tests 1 592 ✅ 57 💤 0 ❌
2 372 runs  2 295 ✅ 77 💤 0 ❌

Results for commit b554434.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@Mikaka27 Mikaka27 force-pushed the michal/erts/handle-windows-short-paths/OTP-19690 branch 6 times, most recently from 60594a2 to 0faeb5e Compare June 26, 2025 09:46
@Mikaka27 Mikaka27 requested a review from dgud June 26, 2025 09:47
@Mikaka27 Mikaka27 marked this pull request as ready for review June 26, 2025 09:48
@Mikaka27 Mikaka27 force-pushed the michal/erts/handle-windows-short-paths/OTP-19690 branch from 0faeb5e to 1cf0dba Compare June 26, 2025 10:20
@dgud dgud requested a review from jhogberg June 26, 2025 13:32
dgud
dgud previously approved these changes Jun 27, 2025
@Mikaka27 Mikaka27 self-assigned this Jun 27, 2025
@Mikaka27 Mikaka27 added the team:PS Assigned to OTP team PS label Jun 27, 2025

/* Chop of base name*/
for (p = dir+wcslen(dir)-1 ;p >= dir && *p != L'\\'; --p)
;
*p =L'\0';
p--;

length = GetLongPathNameW(dir, NULL, 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can get rid of _wcsdup(erlpath); if we chop off the base name after getting the long path name, saving us from having to free(dir) everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that, but here erlpath contains a path to file that can not exist (the erl.ini file), and in that case GetLongPathNameW will fail, if you ask it to convert possibly short path to long path of file/directory that does not exist, as pointed here:

If the function fails for any other reason, such as if the file does not exist, the return value is zero.

That's why I first chop of base name, and only then do the conversion.

free(dir);
error("Cannot find erlexec.dll");
}
long_dir = (wchar_t *) malloc(length * sizeof(wchar_t));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

length does not include the null terminator.

Suggested change
long_dir = (wchar_t *) malloc(length * sizeof(wchar_t));
long_dir = (wchar_t *) malloc((length + 1) * sizeof(wchar_t));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it does include it, here it says:

If the lpBuffer buffer is too small to contain the path, the return value is the size, in TCHARs, of the buffer that is required to hold the path and the terminating null character.

Doesn't it mean that space for null is included in the return we get?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed that to make it more clear, hopefully now it's ok.

@Mikaka27 Mikaka27 force-pushed the michal/erts/handle-windows-short-paths/OTP-19690 branch from 1cf0dba to b554434 Compare June 30, 2025 14:09
@Mikaka27 Mikaka27 requested a review from jhogberg July 1, 2025 08:00
@Mikaka27 Mikaka27 merged commit 98898d4 into erlang:master Jul 4, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:PS Assigned to OTP team PS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants