Skip to content

Fix 6 PVS-Studio static analyzer findings#27035

Open
SufficientDaikon wants to merge 1 commit intoPowerShell:masterfrom
SufficientDaikon:fix/pvs-studio-static-analysis
Open

Fix 6 PVS-Studio static analyzer findings#27035
SufficientDaikon wants to merge 1 commit intoPowerShell:masterfrom
SufficientDaikon:fix/pvs-studio-static-analysis

Conversation

@SufficientDaikon
Copy link
Copy Markdown

@SufficientDaikon SufficientDaikon commented Mar 15, 2026

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