Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix EventPipe utf8 conversion methods to match between JIT and AOT #89353

Merged
merged 1 commit into from
Jul 23, 2023

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Jul 22, 2023

Follow up on #89036 (comment)

Contrary to what the comment says, CoreCLR version of these methods does not allocate the redundant null terminator. Fixing the AOT version of these methods to match.

@ghost
Copy link

ghost commented Jul 22, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Follow up on #89036 (comment)

Contrary to what the comment says, CoreCLR version of these methods does not allocate the redundant null terminator. Fixing the AOT version of these methods to match.

Author: jkotas
Assignees: jkotas
Labels:

area-NativeAOT-coreclr

Milestone: -

@jkotas
Copy link
Member Author

jkotas commented Jul 22, 2023

(We should switch all runtimes to use the AOT version as part of #87069. There is no reason for having runtime-specific versions of these methods.)

Copy link
Contributor

@LakshanF LakshanF left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! I couldn't get this test to work but didn't try out with the 0 length as you suggested (the test passes now!).

@LakshanF LakshanF merged commit ecef854 into dotnet:main Jul 23, 2023
@jkotas jkotas deleted the eventpipe-utf8 branch July 23, 2023 18:38
@ghost ghost locked as resolved and limited conversation to collaborators Aug 22, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants