Skip to content

Add set_max_lol - #77

Closed
gigeresk wants to merge 4 commits into
mainfrom
set_max_lol
Closed

Add set_max_lol#77
gigeresk wants to merge 4 commits into
mainfrom
set_max_lol

Conversation

@gigeresk

@gigeresk gigeresk commented Aug 27, 2026

Copy link
Copy Markdown

Add set_max_lol <int> function.
Sentinel value is -1, so we allow set_max_lol 0.

@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds set_max_lol as persistent SDC metadata and addresses all previously reported round-trip and compatibility defects.

  • Distinguishes unset metadata (-1) from explicitly configured zero.
  • Writes nonnegative values to SDC and preserves the signed value in StaDb snapshots.
  • Bumps the StaDb format version for the new serialized record.

Confidence Score: 5/5

The PR appears safe to merge because the previously reported SDC round-trip and database compatibility failures have been corrected.

No blocking failure remains.

Important Files Changed

Filename Overview
sdc/Sdc.cc Initializes max_lol_ to the -1 sentinel and provides straightforward storage accessors, resolving the previously reported unset-state ambiguity.
sdc/WriteSdc.cc Emits set_max_lol for every configured nonnegative value, preserving explicit zero while omitting the unset sentinel.
stadb/DbFormat.hh Advances the snapshot format version so older and newer binaries reject incompatible SDC encodings at the format boundary.
stadb/DbSdc.cc Symmetrically writes and restores max_lol using the signed integer codec, which preserves the -1 sentinel.
sdc/Sdc.tcl Adds the public command with integer validation that accepts zero as intended.

Reviews (3): Last reviewed commit: "-1 is sentinel value" | Re-trigger Greptile

Comment thread sdc/WriteSdc.cc Outdated
Comment thread stadb/DbSdc.hh
@gigeresk

Copy link
Copy Markdown
Author

@greptile

Comment thread sdc/Sdc.cc Outdated
@gigeresk

Copy link
Copy Markdown
Author

@greptile

@gigeresk
gigeresk requested a review from akashlevy August 27, 2026 21:36
@akashlevy

Copy link
Copy Markdown

Why do we need this in the STA side? Logic levels is computed on Preqorsor side?

@akashlevy akashlevy left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explain why this should be here and not in preqorsor?

@akashlevy

Copy link
Copy Markdown

I think it already is in preqorsor in fact. Please double check this

@gigeresk gigeresk closed this Aug 27, 2026
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.

2 participants