- Notifications
You must be signed in to change notification settings - Fork5.2k
Do not clone empty arrays in CloneByteArray#93231
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ghost commentedOct 9, 2023
Tagging subscribers to this area: @dotnet/area-system-security,@bartonjs,@vcsjones Issue Details
If its empty, just return the identity to save the allocation.
|
Uh oh!
There was an error while loading.Please reload this page.
vcsjones commentedOct 9, 2023
Failure is tracked in#93229. Merging. |
| returnsrcswitch | ||
| { | ||
| returnnull; | ||
| } | ||
| return(byte[])(src.Clone()); | ||
| null=>null, | ||
| {Length:0}=>src, | ||
| _=>(byte[])src.Clone(), | ||
| }; |
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.
FWIW, it could also have been:
returnsrcis notnull?src.AsSpan().ToArray():null;
and let span's ToArray handle the empty case via Array.Empty.
CloneByteArrayis used heavily in S.S.Cryptography to create defensive copies of byte arrays. This is to avoid exposing any kind of mutability to callers.Clonehowever appears tocreate a newbyte[]for empty byte arrays every time. Empty arrays are not uncommon. It could be for a CNG property, it could be for key parameters, or otherAsnEncodedData.If its empty, just return the identity to save the allocation.