Skip to content

Initialize skeleton basic argument storage - #2599

Merged
jwillemsen merged 1 commit into
DOCGroup:masterfrom
jwillemsen:jwi-initialize-sargument-members
Oct 6, 2026
Merged

jwillemsen merged 1 commit into
DOCGroup:masterfrom
jwillemsen:jwi-initialize-sargument-members

Conversation

@jwillemsen

@jwillemsen jwillemsen commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Initialize basic and special-basic skeleton argument-holder storage to prevent indeterminate scalar values from reaching marshaling.

Adds focused CDR coverage for default Boolean and Long return/out holders, including Boolean octet encoding and round-trip unmarshalling.

Validation completed:

  • git diff --check
  • From TAO/tests/CDR: $ACE_ROOT/bin/mwc.pl -type gnuace
  • make -f GNUmakefile.CDR_Basic_Types -j 20
  • ./basic_types -n 1 -l 1

Summary by CodeRabbit

  • Bug Fixes
    • Basic argument, return, and out values now start at zero or false, preventing uninitialized values from reaching marshaling.
  • Tests
    • Added checks for default Boolean and numeric values, including Boolean marshaling and round-trip behavior.

@mergify

mergify Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f3004ff5-e583-486b-b803-9c1ada54ac69
📥 Commits

Reviewing files that changed from the base of the PR and between 74227a0 and 5c6aee4.

📒 Files selected for processing (5)
  • TAO/NEWS
  • TAO/tao/PortableServer/Basic_SArgument_T.h
  • TAO/tao/PortableServer/Special_Basic_SArgument_T.h
  • TAO/tests/CDR/CDR.mpc
  • TAO/tests/CDR/basic_types.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


Walkthrough

Basic and special-basic skeleton argument members now use value initialization. The CDR Basic Types tests check default Boolean and numeric values, Boolean marshaling, and a false Boolean round-trip.

Changes

Skeleton argument initialization

Layer / File(s) Summary
Value-initialize skeleton argument members
TAO/tao/PortableServer/Basic_SArgument_T.h, TAO/tao/PortableServer/Special_Basic_SArgument_T.h, TAO/NEWS
The basic and special-basic argument, return, and out holders now value-initialize their private members. The release notes describe the change.
Check defaults in CDR tests
TAO/tests/CDR/CDR.mpc, TAO/tests/CDR/basic_types.cpp
The Basic Types project adds the portableserver base project. Tests check default Boolean and numeric holder values, Boolean marshaling, and the false Boolean round-trip. The test runner returns failure when these checks fail.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 5c6ae

This change initializes scalar storage in skeleton argument holders, so indeterminate values no longer reach marshaling. The tests cover default and Boolean behavior. No merge-blocking risk was identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: initializing skeleton basic argument storage.
Full details: Docstring Coverage

Explanation

Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks each holder’s state,
With zero values set up straight.
A Boolean hops through CDR,
Then lands as false, just as before.
The rabbit cheers: “No stray bits here!”
And nibbles clover without fear.

Comment @coderabbitai help to get the list of available commands.

@codacy-production

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 1 high

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category Results
ErrorProne 1 high

View in Codacy

🟢 Metrics 15 complexity

Metric Results
Complexity 15

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@jwillemsen
jwillemsen merged commit 451626a into DOCGroup:master Oct 6, 2026
45 of 48 checks passed
@jwillemsen
jwillemsen deleted the jwi-initialize-sargument-members branch October 6, 2026 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant