Skip to content

Conversation

rcj1
Copy link
Contributor

@rcj1 rcj1 commented Jul 10, 2025

No description provided.

@Copilot Copilot AI review requested due to automatic review settings July 10, 2025 20:42
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a managed implementation of GetDomainFromContext in SOSDacImpl with basic argument validation, assignment logic, and debug-only comparisons against the legacy DAC.

  • Introduces a full-method body replacing the one-liner forward to _legacyImpl
  • Validates inputs and sets the output pointer, capturing exceptions as HRESULTs
  • Adds debug-only assertions to compare results with the legacy implementation
Comments suppressed due to low confidence (1)

src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs:247

  • Add unit tests to cover the invalid-argument path (when context is zero or domain is null) and the exception path to verify the correct HRESULT is returned.
        if (context == 0 || domain == null)

Copy link
Contributor

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

Copy link
Member

@max-charlamb max-charlamb left a comment

Choose a reason for hiding this comment

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

Looks good mod that one comment

@rcj1 rcj1 merged commit 956aba4 into dotnet:main Jul 11, 2025
45 of 47 checks passed
@rcj1 rcj1 deleted the GetDomainFromContext branch July 11, 2025 23:51
@rcj1 rcj1 restored the GetDomainFromContext branch July 11, 2025 23:55
@rcj1 rcj1 deleted the GetDomainFromContext branch July 11, 2025 23:57
@github-actions github-actions bot locked and limited conversation to collaborators Aug 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants