Skip to content

Fix 6 PVS-Studio static analyzer findings#27035

Open
SufficientDaikon wants to merge 2 commits intoPowerShell:masterfrom
SufficientDaikon:fix/pvs-studio-static-analysis
Open

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

Conversation

@SufficientDaikon
Copy link

Reviewer shortcut — For a structured walkthrough of all three related contributions,
see the companion review site — easier to scan than this PR body.


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.


What Changed

Item File Bug Severity Fix
9 StringUtil.cs ?? precedence — appendStr length silently ignored Medium Added parentheses
2 CimCmdlets/Utils.cs Unsafe double-checked locking Low Added volatile
6 ConsoleHost.cs RunspaceRef used before null check High Moved null check first
1 CimGetInstance.cs GetCimInstanceParameter can return null High Added null guard
5 ShowCommandCommandInfo.cs Members["Module"] without null-conditional Medium Added ?.
7 typeDataQuery.cs vd.mainControl accessed when vd == null High Split into separate checks

Items NOT Fixed

Intentionally left for maintainer guidance:

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
10 Command.cs PipelineResultTypes Flags enum — changing values is a breaking change

Tests

  • Build: clean compile, 0 errors across all 6 changed files
  • These are defensive null-checks and correctness fixes in rare/edge-case code paths. The bugs would manifest as NullReferenceException crashes or silent miscalculation under specific conditions.

Before / After for each item (click to expand)

Item 9 — Operator precedence

// Before — ?? has lower priority than +
int capacity = length + prependStr?.Length ?? 0 + appendStr?.Length ?? 0;

// After
int capacity = length + (prependStr?.Length ?? 0) + (appendStr?.Length ?? 0);

Item 2 — Missing volatile

// Before
private static bool logInitialized = false;

// After
private static volatile bool logInitialized = false;

Item 6 — Null check order

// Before — RunspaceRef used before null check
if (_isRunspacePushed)
    return RunspaceRef.OldRunspace as LocalRunspace;
if (RunspaceRef == null) return null;

// After — null check first
if (RunspaceRef == null) return null;
if (_isRunspacePushed)
    return RunspaceRef.OldRunspace as LocalRunspace;

Item 1 — Null dereference

// Before
CimInstance instance = GetCimInstanceParameter(cmdlet);
nameSpace = instance.CimSystemProperties.Namespace;

// After
CimInstance instance = GetCimInstanceParameter(cmdlet);
if (instance != null)
{
    nameSpace = instance.CimSystemProperties.Namespace;
    ...
}

Item 5 — Null access

// Before
this.Module = other.Members["Module"].Value as ShowCommandModuleInfo;

// After
this.Module = other.Members["Module"]?.Value as ShowCommandModuleInfo;

Item 7 — Null dereference in trace

// Before — vd.mainControl accessed when vd == null
if (vd == null || mainControlType != vd.mainControl.GetType())
{
    ActiveTracer.WriteLine("NOT MATCH {0}", ControlBase.GetControlShapeName(vd.mainControl), ...);
}

// After — separate checks
if (vd == null)
{
    ActiveTracer.WriteLine("NOT MATCH null view definition");
    continue;
}
if (mainControlType != vd.mainControl.GetType()) { ... }

Related

Addresses #25289 (items 1, 2, 5, 6, 7, 9)

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 <[email protected]>
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.

1 participant