-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Return WTF-16 from W-suffixed functions instead of converting to WTF-8 #23657
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
base: master
Are you sure you want to change the base?
Conversation
19aacf3
to
74a493e
Compare
To follow the standard deprecation procedure:
Here's a previous example: #19844 |
I see that Regardless, here are some preliminary notes about changes to that function: I think we would want to modify All in all, I believe that would require deprecating and adding replacements for 5 functions:
|
If the readLink changes are more involved, might be better to do those in a separate PR. |
1bc5893
to
5ac4113
Compare
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.
Great work, noticing/utilizing the ability to re-use the buffer for input/output of realpathW2 is a nice touch.
(the CI failure is unrelated to this PR, will re-run it once the CI server is restarted)
5ac4113
to
2569835
Compare
- Rename modified `realpathW` to `realpathW2` - Re-add original `realpathW` - Add deprecation notice to `realpathW`
2569835
to
fcf8123
Compare
(no need to continually rebase btw, if the CI is all green then you can just wait for a core Zig team member to review it. If there's an unrelated failure on one or two CI runners, then those can be manually restarted without restarting all the rest of the runners [but all the runners need to have finished before the failing one(s) can be restarted]) |
This was suggested here and is another step towards fixing #23643.
These changes decrease the stack usage of
realpathW
by 64 KB.I took the liberty of additionally documenting that
realpathW
will never access thepathname
after it starts writing toout_buffer
and took advantage of that to avoid moving the WTF-16 buffer into many call-sites. If this is undesirable, I can remove those particular changes.As an aside, I noticed that
QueryObjectName
silently aligns up its buffer to@alignOf(OBJECT_NAME_INFORMATION)
(which has the same alignment as pointers). Should this be documented on the function and in functions that call it so that callers know that using astd.os.windows.PATH_MAX_WIDE
length buffer could technically fail witherror.NameTooLong
?