GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix 6 PVS-Studio static analyzer findings by SufficientDaikon · Pull Request #27035 · PowerShell/PowerShell

Conversation

@SufficientDaikon

@SufficientDaikon Ahmed Taha (SufficientDaikon) commented Mar 15, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

PR Summary

Fixes 6 of the 10 PVS-Studio static analyzer findings reported in #25289. The remaining 4 items require design decisions, affect auto-generated code, or would be breaking changes — rationale below.

fixed skipped high medium low

Fixed Items

Item File Bug Severity Fix
6 ConsoleHost.cs RunspaceRef used before null check High Moved null check first
1 CimGetInstance.cs GetCimInstanceParameter can return null, dereferenced High Added null guard
7 typeDataQuery.cs vd.mainControl accessed when vd == null High Split into separate checks
9 StringUtil.cs ?? operator precedence — appendStr length silently ignored Medium Added parentheses
5 ShowCommandCommandInfo.cs Members["Module"] without null-conditional Medium Added ?.
2 CimCmdlets/Utils.cs Unsafe double-checked locking Low Added volatile
Item 6 — Null check order in ConsoleHost.cs (High)
  // RunspaceRef used before null check
- if (_isRunspacePushed)
-     return RunspaceRef.OldRunspace as LocalRunspace;
- if (RunspaceRef == null) return null;
+ if (RunspaceRef == null) return null;
+ if (_isRunspacePushed)
+     return RunspaceRef.OldRunspace as LocalRunspace;
Item 1 — Null dereference in CimGetInstance.cs (High)
  CimInstance instance = GetCimInstanceParameter(cmdlet);
- nameSpace = instance.CimSystemProperties.Namespace;
+ if (instance != null)
+ {
+     nameSpace = instance.CimSystemProperties.Namespace;
+     ...
+ }
Item 7 — Null dereference in typeDataQuery.cs (High)
  // vd.mainControl accessed when vd == null
- if (vd == null || mainControlType != vd.mainControl.GetType())
- {
-     ActiveTracer.WriteLine("NOT MATCH ...",
-         ControlBase.GetControlShapeName(vd.mainControl), ...);
- }
+ if (vd == null)
+ {
+     ActiveTracer.WriteLine("NOT MATCH null view definition");
+     continue;
+ }
+ if (mainControlType != vd.mainControl.GetType())
+ {
+     ActiveTracer.WriteLine("NOT MATCH ...", ...);
+     continue;
+ }
Item 9 — Operator precedence in StringUtil.cs (Medium)
  // ?? has lower priority than +
- int capacity = length + prependStr?.Length ?? 0 + appendStr?.Length ?? 0;
+ int capacity = length + (prependStr?.Length ?? 0) + (appendStr?.Length ?? 0);
Item 5 — Null access in ShowCommandCommandInfo.cs (Medium)
- this.Module = other.Members["Module"].Value as ShowCommandModuleInfo;
+ this.Module = other.Members["Module"]?.Value as ShowCommandModuleInfo;
Item 2 — Missing volatile in Utils.cs (Low)
- private static bool logInitialized = false;
+ private static volatile bool logInitialized = false;

Skipped Items

Note

4 items intentionally left for maintainer guidance. Each requires a design decision beyond a mechanical fix.

Skipped items with rationale
Item File Why Skipped
3 GetEventCommand.cs suppressOpener has no format placeholders — may be intentional design
4 New-Object.cs Missing FullLanguage switch case — may be deliberate fall-through
8 xmlSerializer.autogen.cs Auto-generated file — fix would be overwritten on next generation
10 Command.cs PipelineResultTypes Flags enum values — changing is a public API breaking change

PR Context

Addresses #25289 (items 1, 2, 5, 6, 7, 9). These are defensive null-checks and correctness fixes in code paths that would manifest as NullReferenceException crashes or silent miscalculation under specific conditions.


PR Checklist

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
  • This PR is ready to merge. If this PR is a work in progress, please open this as a Draft Pull Request and mark it as Ready to Review when it is ready to merge.
  • Breaking changes
    • None
  • User-facing changes
    • Not Applicable
  • Testing - New and feature
    • N/A or can only be tested interactively

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@SufficientDaikon
Ahmed Taha (SufficientDaikon) requested a review from a team as a code owner March 15, 2026 20:51
@SufficientDaikon Ahmed Taha (SufficientDaikon) mentioned this pull request Mar 15, 2026
Some issues found by static analyzer #25289
Open
5 tasks done
@SufficientDaikon @claude
Fix 6 PVS-Studio static analyzer findings
881d2bb
Address items 1, 2, 5, 6, 7, 9 from the PVS-Studio report in PowerShell#25289:

- Item 9: Fix ?? operator precedence in StringUtil.cs — appendStr
  length was silently ignored in capacity calculation
- Item 2: Add volatile to logInitialized in CimCmdlets/Utils.cs
  for safe double-checked locking
- Item 6: Move RunspaceRef null check before first use in
  ConsoleHost.cs to prevent NullReferenceException
- Item 1: Add null guard for GetCimInstanceParameter return value
  in CimGetInstance.cs
- Item 5: Use null-conditional on Members["Module"] access in
  ShowCommandCommandInfo.cs
- Item 7: Split vd null check from type check in typeDataQuery.cs
  to prevent NullReferenceException in trace logging

Items 3, 4, 8, 10 are intentionally left — they require design
decisions, affect auto-generated code, or would be breaking changes.

Addresses PowerShell#25289

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@SufficientDaikon
Ahmed Taha (SufficientDaikon) force-pushed the fix/pvs-studio-static-analysis branch from 514fb65 to 881d2bb Compare March 24, 2026 15:40
This was referenced Mar 24, 2026
Add bounded-wait timeout support to hosting API #27027
Open
Change New-Guid to generate UUID v7 by default #27033
Merged
Fix Get-Culture -ListAvailable:$false and Get-Location -Stack:$false #27034
Open
Fix get-changed-files action for schedule and workflow_dispatch events #27078
Open
@microsoft-github-policy-service microsoft-github-policy-service Bot added the Review - Needed The PR is being reviewed label Apr 1, 2026
@microsoft-github-policy-service

microsoft-github-policy-service Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@SufficientDaikon
Merge branch 'master' into fix/pvs-studio-static-analysis
a9c35ee
Copilot AI review requested due to automatic review settings July 22, 2026 14:23
Copilot started reviewing on behalf of Ahmed Taha (SufficientDaikon) July 22, 2026 14:24
View session
Copilot AI reviewed Jul 22, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a subset of the PVS-Studio findings from #25289 by applying targeted defensive fixes (null guards, corrected operator precedence, and a thread-safety adjustment) across several PowerShell components to prevent NullReferenceExceptions and subtle correctness issues.

Changes:

  • Fixed potential null dereferences in ConsoleHost, type data view selection, CIM instance retrieval, and Show-Command object rehydration.
  • Corrected ?? operator precedence in StringUtil.VtSubstring capacity calculation.
  • Improved safety of a double-checked locking pattern by marking the guard flag as volatile.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/System.Management.Automation/utils/StringUtil.cs Fixes ??/+ precedence to compute StringBuilder capacity correctly.
src/System.Management.Automation/FormatAndOutput/common/DisplayDatabase/typeDataQuery.cs Splits null checks to avoid dereferencing vd when it is null and keeps tracing safe.
src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs Reorders checks to avoid using RunspaceRef before verifying it is non-null.
src/Microsoft.PowerShell.Commands.Utility/commands/utility/ShowCommand/ShowCommandCommandInfo.cs Uses null-conditional member access to avoid null dereference on missing "Module" member.
src/Microsoft.Management.Infrastructure.CimCmdlets/Utils.cs Marks logInitialized as volatile to make double-checked locking safer.
src/Microsoft.Management.Infrastructure.CimCmdlets/CimGetInstance.cs Guards CimInstance usage to prevent dereference when GetCimInstanceParameter returns null.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Review - Needed The PR is being reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@SufficientDaikon
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.