Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix 6 PVS-Studio static analyzer findings#27035

Open
SufficientDaikon wants to merge 1 commit into
PowerShell:masterPowerShell/PowerShell:masterfrom
SufficientDaikon:fix/pvs-studio-static-analysisSufficientDaikon/PowerShell:fix/pvs-studio-static-analysisCopy head branch name to clipboard
Open

Fix 6 PVS-Studio static analyzer findings#27035
SufficientDaikon wants to merge 1 commit into
PowerShell:masterPowerShell/PowerShell:masterfrom
SufficientDaikon:fix/pvs-studio-static-analysisSufficientDaikon/PowerShell:fix/pvs-studio-static-analysisCopy head branch name to clipboard

Conversation

@SufficientDaikon

@SufficientDaikon SufficientDaikon commented Mar 15, 2026

Copy link
Copy Markdown

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

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 <noreply@anthropic.com>
@microsoft-github-policy-service

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

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

Labels

Review - Needed The PR is being reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Morty Proxy This is a proxified and sanitized view of the page, visit original site.