Removed unsafe string mutation #31850
Conversation
ghost
added
the
community-contribution
| @@ -65,25 +65,22 @@ public BCryptKeyHandle GenerateSymmetricKey(byte* pbSecret, uint cbSecret) | |||
| /// </summary> | |||
| public string GetAlgorithmName() | |||
| { | |||
| const int StackAllocCharSize = 128; | |||
There was a problem hiding this comment.
There can't be longer algorithm names?
I gues no, but ... want to double-check.
There was a problem hiding this comment.
I'd be extremely surprised if that were the case. So what you had proposed should be good.
If you wanted to be defensive you could always string.Create instead of stackalloc, but overflowing this buffer is never going to happen in practice. (This method isn't even called outside unit tests TBH.)
| @@ -70,27 +67,59 @@ private static unsafe void GetAsciiStringNonNullCharacters(Span<char> buffer, In | |||
| } | |||
| } | |||
|
|
|||
| private static readonly SpanAction<char, IntPtr> s_getAsciiStringNonNullCharacters = GetAsciiStringNonNullCharacters; | |||
There was a problem hiding this comment.
Nit: Can we move the static SpanAction fields to the top of the class?
There was a problem hiding this comment.
Done with efe80e7
A note for the rationale behind the placement:
As these delegates are used in only one special place (to avoid the compiler generating the cachable display-class and the null-check) these were placed near to the usage to have "compact code".
|
Thanks @gfoidl! |
amcasey
added
area-networking
Addresses #31821
I didn't ran benchmarks, as it's basically the same as in #17556 and there this pattern is an improvement.
These should cover all occurances of mutating strings, based on
new string\(('\\0'|\(char\)0)-search./cc: @GrabYourPitchforks PTAL