Skip to content

DynamicDependencies: fix PCWSTR[] access#872

Merged
DrusTheAxe merged 2 commits intomainfrom
user/drustheaxe/array-cert
May 27, 2021
Merged

DynamicDependencies: fix PCWSTR[] access#872
DrusTheAxe merged 2 commits intomainfrom
user/drustheaxe/array-cert

Conversation

@DrusTheAxe
Copy link
Copy Markdown
Member

Also fixed cert access in DevCheck.ps1 causing false errors

@DrusTheAxe DrusTheAxe added bug Something isn't working area-DynamicDependencies DynamicDependency for Windows App SDK: enables framework packages for packaged & unpackaged apps labels May 27, 2021
@DrusTheAxe DrusTheAxe added this to the 0.8 (2021 Q2) milestone May 27, 2021
@DrusTheAxe DrusTheAxe self-assigned this May 27, 2021
@ghost ghost added the needs-triage label May 27, 2021
@jonwis
Copy link
Copy Markdown
Member

jonwis commented May 27, 2021

Is there a test that was failing, and now won't be?

What issue # are you fixing?

Copy link
Copy Markdown
Member

@jonwis jonwis left a comment

Choose a reason for hiding this comment

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

Minor comments

Comment on lines +47 to 48
const auto packageFullName{ packageFullNames[index] };
packageFullNamesList.push_back(std::wstring(packageFullName));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const auto packageFullName{ packageFullNames[index] };
packageFullNamesList.push_back(std::wstring(packageFullName));
packageFullNamesList.emplace_back(packageFullNames[index]);

Comment thread tools/DevCheck.ps1
@DrusTheAxe
Copy link
Copy Markdown
Member Author

Is there a test that was failing, and now won't be?
Wasn't noticed due to a test gap. Closing that gap is tricky so won't happen this week (I'm on VACA and traveling, just happened to see the issue and have a few...). But @AdamBraden has an easy manual repro,

What issue # are you fixing?
@AdamBraden just created one. Microsoft-internal task 33487794

@DrusTheAxe DrusTheAxe enabled auto-merge (squash) May 27, 2021 04:52
@BenJKuhn
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@DrusTheAxe DrusTheAxe merged commit 5f7911e into main May 27, 2021
@DrusTheAxe DrusTheAxe deleted the user/drustheaxe/array-cert branch May 27, 2021 05:36
BenJKuhn pushed a commit that referenced this pull request May 27, 2021
* Fixed cert enumeration (incorrect probing caused failures)

* Fixed incorrect PCWSTR[] access
BenJKuhn added a commit that referenced this pull request May 27, 2021
* Fixed cert enumeration (incorrect probing caused failures)

* Fixed incorrect PCWSTR[] access

Co-authored-by: Howard Kapustein <howardk@microsoft.com>
DrusTheAxe added a commit that referenced this pull request Jun 14, 2021
* Fixed cert enumeration (incorrect probing caused failures)

* Fixed incorrect PCWSTR[] access
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-DynamicDependencies DynamicDependency for Windows App SDK: enables framework packages for packaged & unpackaged apps bug Something isn't working needs-triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants