Skip to content

Clean up some code in ANCM #49718

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

Merged
merged 2 commits into from
Jul 31, 2023

Conversation

adityamandaleeka
Copy link
Member

Cleaning up some stuff I noticed while I was looking into ANCM code. This includes some dead code removal, fixing weird conditionals, removing some gotos, etc.

This is pure cleanup and should have no functional impact.

cc: @BrennanConroy

}
else if (struPath.QueryStr()[dwPosition-1] == L':')

if (struPath.QueryStr()[position - 1] == L':')
Copy link
Member

Choose a reason for hiding this comment

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

You removed the position == 0 condition so this one can buffer underrun now

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed it because it can never be 0. Can it? Even in the case where we start at 0, the STRU::IndexOf is given the start offset of position+1 to search from, and it will either return -1 (if not found) or a position >=1.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I guess. Probably add a comment then since it wasn't obvious from first glance

HRESULT
FILE_UTILITY::EnsureDirectoryPathExist(
_In_ LPCWSTR pszPath
FILE_UTILITY::EnsureDirectoryPathExists(
Copy link
Member

Choose a reason for hiding this comment

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

Need to change the header and caller

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, I used the VS refactor thing and forgot to save the other modified files 😆

@adityamandaleeka adityamandaleeka enabled auto-merge (squash) July 31, 2023 17:47
@adityamandaleeka adityamandaleeka merged commit cf006b0 into dotnet:main Jul 31, 2023
@ghost ghost added this to the 8.0-rc1 milestone Jul 31, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants