Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Replace a few stackallocs with collection expressions #105121

Merged
merged 1 commit into from
Jul 19, 2024
Merged

Replace a few stackallocs with collection expressions #105121

merged 1 commit into from
Jul 19, 2024

Conversation

stephentoub
Copy link
Member

No description provided.

@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

@danmoseley danmoseley merged commit 87cd893 into dotnet:main Jul 19, 2024
3 checks passed
@stephentoub
Copy link
Member Author

Thanks, @danmoseley, but CI is down and didn't run on this pr

@danmoseley
Copy link
Member

Oh nuts... was there a way I could have determined that? I thought it was all green - but I'm on a phone.

@danmoseley
Copy link
Member

Oh, expanding it I see the legs are missing.

@tannergooding
Copy link
Member

I think this getting merged introduce a build break:

src\libraries\System.Security.Cryptography\src\System\Security\Cryptography\Pbkdf2Implementation.Windows.cs(127,25): error CS0165: (NETCORE_ENGINEERING_TELEMETRY=Build) Use of unassigned local variable 'symmetricKeyMaterialLength'

kunalspathak
kunalspathak reviewed Jul 19, 2024
View reviewed changes
...ystem.Security.Cryptography/src/System/Security/Cryptography/Pbkdf2Implementation.Windows.cs
@@ -60,8 +60,7 @@ internal static partial class Pbkdf2Implementation
if (password.IsEmpty)
{
// CNG won't accept a null pointer for the password.
symmetricKeyMaterial = stackalloc byte[1];
symmetricKeyMaterialLength = 0;
Copy link
Member

Choose a reason for hiding this comment

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

@stephentoub - this change is causing build errors:

D:\a\_work\1\s\src\libraries\System.Security.Cryptography\src\System\Security\Cryptography\Pbkdf2Implementation.Windows.cs(127,25): error CS0165: Use of unassigned local variable 'symmetricKeyMaterialLength' [D:\a\_work\1\s\src\libraries\System.Security.Cryptography\src\System.Security.Cryptography.csproj::TargetFramework=net9.0-windows]

https://dev.azure.com/dnceng-public/public/_build/results?buildId=747203&view=logs&j=d2b92d58-337d-5541-2ecb-d35f35f29352&t=3bcb3c60-9dac-5f6d-6a96-02987273246b

Copy link
Member

Choose a reason for hiding this comment

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

Opened #105126

@kunalspathak kunalspathak mentioned this pull request Jul 19, 2024
Closed
jkotas added a commit that referenced this pull request Jul 19, 2024
@jkotas
Revert "Replace a few stackallocs with collection expressions (#105121)"
e6339c6 
This reverts commit 87cd893.
@jkotas jkotas mentioned this pull request Jul 19, 2024
Merged
jkotas added a commit that referenced this pull request Jul 19, 2024
@jkotas
Revert "Replace a few stackallocs with collection expressions (#105121)…
57a2712 
…" (#105128)

This reverts commit 87cd893.
@jkotas
Copy link
Member

jkotas commented Jul 19, 2024

Reverted in #105128. @stephentoub Could you please resubmit?

@stephentoub stephentoub deleted the stackallocce branch July 19, 2024 10:23
@danmoseley
Copy link
Member

Apologies, that was my fault (for not reverting).

@github-actions github-actions bot locked and limited conversation to collaborators Aug 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kunalspathak kunalspathak kunalspathak left review comments

@danmoseley danmoseley danmoseley approved these changes

Assignees

@stephentoub stephentoub

Labels
area-Meta
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@stephentoub @danmoseley @tannergooding @jkotas @kunalspathak