Remove Marshal Exception/HRESULT proxies from SecurityHelper, simplify code#9977
Remove Marshal Exception/HRESULT proxies from SecurityHelper, simplify code#9977h3xds1nz wants to merge 7 commits into
Conversation
dotnet-policy-service
Bot
requested review from
dipeshmsft and
singhashish-wpf
dotnet-policy-service
Bot
added
PR
harshit7962
added
Status:Completed
and removed
Status:InProgress
Status:Proposed
labels
|
@h3xds1nz this PR is good to be merged. Can you please resolve the merge conflicts? |
h3xds1nz
force-pushed
the
remove-securityhelper-exceptions
branch
from
03a4164 to
0c13826
Compare
|
@himgoyalmicro Done :) |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9977 +/- ##
===================================================
+ Coverage 11.25778% 12.83462% +1.57683%
===================================================
Files 3315 3316 +1
Lines 665229 665427 +198
Branches 74668 74667 -1
===================================================
+ Hits 74890 85405 +10515
+ Misses 589035 577806 -11229
- Partials 1304 2216 +912
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Runtime.InteropServices; |
There was a problem hiding this comment.
Can you sort the usings ?
There was a problem hiding this comment.
Almost none (or none?) of the files that I've touched has the usings sorted to begin with, don't think I should.
Plus then there's #10683 (comment) to tackle.
There was a problem hiding this comment.
Personally a lot of the files I've seen were sorted, or mostly sorted. It's also pretty standard to sort usings in C#. This file was already sorted so why unsort it ?
|
|
||
| #if PRESENTATION_CORE | ||
| using MS.Internal.AppModel; | ||
| using System.Security; |
There was a problem hiding this comment.
Can you sort the usings ?
There was a problem hiding this comment.
See the other comment. Also with tiny amount of luck this file won't exist in .NET 10 RTM anyways.
There was a problem hiding this comment.
See my other comment. This file was not fully sorted but I don't see why we should add unsorted usings.
4 participants
Description
Continues the work on removing CAS remnants, this time again method proxies and "helpers" from
SecurityHelper.In this PR we remove
GetHRForExceptionfromStreamAsIStreamalong with obsolete warning suppresions and alsoGetExceptionForHRplusThrowExceptionForHRproxies._lastExceptionwas just storing the latest exception in a private field that was never read.COR_E_OBJECTDISPOSEDin some cases instead of setting up a frame, conditionally throwingObjectDisposedException, catching it just to retrieve theHResultand then returning it but I think it is out of scope for this PR.Customer Impact
Decreased size of assemblies, cleaner codebase for developers.
Regression
No.
Testing
Local build.
Risk
Low.
Microsoft Reviewers: Open in CodeFlow