Skip to content

os/Transaction: initialize unused fields in TransactionData#65075

Merged
tchaikov merged 1 commit intoceph:mainfrom
tchaikov:wip-transaction-set-unused
Aug 21, 2025
Merged

os/Transaction: initialize unused fields in TransactionData#65075
tchaikov merged 1 commit intoceph:mainfrom
tchaikov:wip-transaction-set-unused

Conversation

@tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Aug 18, 2025

Initialize unused1, unused2, and unused3 fields to zero in TransactionData to ensure consistent encoding/decoding behavior.

Background:
In commit a0c9fec, we updated TransactionData encoding/decoding and bumped the Transaction encoding version from 9 to 10. As part of this change, we renamed three fields to mark them as unused:

  • largest_data_len → unused1
  • largest_data_off → unused2
  • largest_data_off_in_data_bl → unused3

The move constructor was also updated to stop setting these fields, leaving them uninitialized after move operations.

Problem:
This worked with existing tests because check-generated.sh reused struct instances, preserving stale values across encode/decode cycles. However, an upcoming test change will stop reusing instances and compare hexdumps of encoded/re-encoded values to verify consistency. Uninitialized fields cause these comparisons to fail due to garbage values.

Solution:
Initialize the unused fields to zero in the move constructor. This preserves existing behavior while ensuring consistent encoding. These fields can be removed entirely in a future change.

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

@tchaikov
Copy link
Contributor Author

tchaikov commented Aug 18, 2025

see #63910 for the upcoming change which stops reusing the same instances for dencoder testing.

@tchaikov
Copy link
Contributor Author

jenkins test make check

@tchaikov
Copy link
Contributor Author

jenkins test make check arm64

@tchaikov
Copy link
Contributor Author

jenkins test make check

Initialize unused1, unused2, and unused3 fields to zero in TransactionData
to ensure consistent encoding/decoding behavior.

Background:
In commit a0c9fec, we updated TransactionData encoding/decoding and bumped
the Transaction encoding version from 9 to 10. As part of this change, we
renamed three fields to mark them as unused:
- largest_data_len → unused1
- largest_data_off → unused2
- largest_data_off_in_data_bl → unused3

The move constructor was also updated to stop setting these fields, leaving
them uninitialized after move operations.

Problem:
This worked with existing tests because check-generated.sh reused struct
instances, preserving stale values across encode/decode cycles. However,
an upcoming test change will stop reusing instances and compare hexdumps
of encoded/re-encoded values to verify consistency. Uninitialized fields
cause these comparisons to fail due to garbage values.

Solution:
Initialize the unused fields to zero in the move constructor. This preserves
existing behavior while ensuring consistent encoding. These fields can be
removed entirely in a future change.

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
@tchaikov
Copy link
Contributor Author

FWIW, i temporarily included #64273 to see if it addressed the test failures surfaced when testing this pull request, and it did. so i am removing it from this pull request.

@tchaikov
Copy link
Contributor Author

jenkins test make check

@tchaikov tchaikov merged commit 088180d into ceph:main Aug 21, 2025
13 checks passed
@tchaikov tchaikov deleted the wip-transaction-set-unused branch August 21, 2025 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants