-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Rewrite SpanHelpers.IndexOfNullByte/IndexOfNullCharacter to unmanaged pointers #81347
Conversation
Tagging subscribers to this area: @dotnet/area-system-memory Issue DetailsAddresses #81332
|
|
||
internal static unsafe int strlen(byte* ptr) => SpanHelpers.IndexOfNullByte(ref *ptr); | ||
internal static unsafe int strlen(byte* ptr) => SpanHelpers.IndexOfNullByte(ptr); |
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.
a nit: we can use SpanHelpers.IndexOfNullByte
directly at call-sites and delete these wrappers to promote the usage of same API everywhere (CoreLib has total 21 call-sites of wcslen
and strlen
combined).
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.
a nit: we can use
SpanHelpers.IndexOfNullByte
directly at call-sites and delete these wrappers to promote the usage of same API everywhere (CoreLib has total 21 call-sites ofwcslen
andstrlen
combined).
It's a bit more complicated too delete it - e.g. VM relies on it for IL marshallers, mono seems too
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.
Ah corelib.h, forgot to check that. 😁
(they are used in ilmarshalers.cpp
via METHOD__STRING__STRLEN
and METHOD__STRING__WCSLEN
macros)
Calls in C# code can use SpanHelpers directly.
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.
Calls in C# code can use SpanHelpers directly.
I wonder if in this case ILLink will trim strlen
/the other one as unused and then VM will crash, so probably better to leave as is or rename completely?
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.
For CoreCLR, ILLink keeps everything referenced by VM. I am not sure about Mono.
Addresses #81332