Skip to content

Conversation

@daxian-dbw
Copy link
Member

@daxian-dbw daxian-dbw commented Jul 24, 2018

PR Summary

Quote this comment from the existing Utils.IsComObject:

// We can't use System.Runtime.InteropServices.Marshal.IsComObject(obj) since it doesn't work in partial trust.

Partial trust is a concept of Code Access Security (CAS)/Security-Transparent. CAS and Security-Transparent are no longer supported in .NET Core, so we should use Marshal.IsComObject instead.

PR Checklist

@daxian-dbw daxian-dbw requested a review from BrucePay as a code owner July 24, 2018 00:32
@daxian-dbw daxian-dbw added the Issue-Code Cleanup the issue is for cleaning up the code with no impact on functionality label Jul 24, 2018

internal static readonly MethodInfo Utils_IsComObject =
typeof(Utils).GetMethod(nameof(Utils.IsComObject), staticFlags, binder: null, types: new Type[] {typeof(object)}, modifiers: null);
typeof(Utils).GetMethod(nameof(Utils.IsComObject), staticFlags);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems it is single place where we use the Utils.IsComObject method. Could you please clarify why we can not use Marshal.IsComObject here? Null check?

Copy link
Member Author

Choose a reason for hiding this comment

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

Marshal.IsComObject cannot accept null input -- it throws ArgumentNullException if the argument is null. Utils.IsComObject handles null input properly. For the rest call sites where I replaced with Marshal.IsComObject, the input object is guaranteed to be non-null.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@daxian-dbw daxian-dbw merged commit 41111a1 into PowerShell:master Jul 26, 2018
@daxian-dbw daxian-dbw deleted the iscomobject branch July 26, 2018 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Issue-Code Cleanup the issue is for cleaning up the code with no impact on functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants