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.

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

Fixed Enumerator variable null check #89920

Merged

Conversation

PabloKharo
Copy link
Contributor

@PabloKharo PabloKharo commented Aug 3, 2023

Fixed error when _hashtableContentsToEnumerate is null.
Error will always appear when this variable is null when using 'Length' property.

Also reduced nesting.

The error was detected using the Svace analyzer maded by ISP RAS.

Fixed error when _hashtableContentsToEnumerate is null.
It appears always when using 'Length' property when variable is null.
Also reduced nesting.
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Aug 3, 2023
@PabloKharo
Copy link
Contributor Author

@dotnet-policy-service agree

Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

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

Thanks!

@VSadov could you also have a look? I'm always afraid when I see lock free because I know enough about lock free programming to understand I'm not qualified to write or review such code.

@PabloKharo
Copy link
Contributor Author

Hi, are there any updates on my pull request?

@VSadov
Copy link
Member

VSadov commented Aug 10, 2023

I will take a look today.

@VSadov
Copy link
Member

VSadov commented Aug 11, 2023

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@VSadov
Copy link
Member

VSadov commented Aug 12, 2023

@PabloKharo - looks good to me. I just want to run a few more tests to be sure.

Copy link
Member

@VSadov VSadov left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!!

@VSadov VSadov merged commit 32a25db into dotnet:main Aug 12, 2023
153 of 163 checks passed
@PabloKharo PabloKharo deleted the _hashtableContentsToEnumerate_null_check branch August 13, 2023 14:46
@ghost ghost locked as resolved and limited conversation to collaborators Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-crossgen2-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants