8000 Void codemethod call should not crash by powercode · Pull Request #4850 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Void codemethod call should not crash #4850

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 8000 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 3 commits into from
Sep 20, 2017

Conversation

powercode
Copy link
Collaborator

Fixes #4826

Adding a check if the method to be called is void, and in that case wrap it in a block that returns null.

Looking at some other code, there seems to be a CSharpBinderFlags.ResultDiscarded when they to this in C#. Can reviewers please check if this or something similar needs to be used here?

Copy link
Contributor
@lzybkr lzybkr left a comment

Choose a reason for hiding this comment

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

Just one small change and I think it's good.

Expression expr = InvokeMethod(codeMethod.CodeReference, null, args.Prepend(target).ToArray(), false, MethodInvocationType.Ordinary);
if (codeMethod.CodeReference.ReturnType == typeof(void))
{
expr = Expression.Block(expr, Expression.Default(typeof(object)));
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be:

                    expr = Expression.Block(expr, ExpressionCache.AutomationNullConstant);

Otherwise it might be possible to use the return value.

@lzybkr lzybkr merged commit 916ef56 into PowerShell:master Sep 20, 2017
@vors vors removed their request for review September 25, 2017 00:33
@powercode powercode deleted the void_codemethod branch January 26, 2018 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0