Skip to content

register: updated write/read debug function to support side effect#136

Merged
eyck merged 1 commit into
Minres:mainfrom
quic-prateek:register_transport_dbg_behavior
Jun 5, 2026
Merged

register: updated write/read debug function to support side effect#136
eyck merged 1 commit into
Minres:mainfrom
quic-prateek:register_transport_dbg_behavior

Conversation

@quic-prateek

Copy link
Copy Markdown
Contributor

No description provided.

@quic-prateek quic-prateek requested a review from a team as a code owner June 2, 2026 17:08
@eyck

eyck commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Thanks @quic-prateek fro the PR.
Can you make the behavior configurable e.g. by an cci_param? Having side effects is in many cases fine. But if I have a register with clear-on-read behavior, a debugger access would change the state.

@eyck eyck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please make the behavior configurable by a cci_param to disable side effects

@quic-prateek quic-prateek force-pushed the register_transport_dbg_behavior branch from 4c80aeb to cb97c44 Compare June 3, 2026 05:33
@quic-prateek quic-prateek force-pushed the register_transport_dbg_behavior branch from cb97c44 to f25b1c0 Compare June 3, 2026 05:40
@quic-prateek

quic-prateek commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Please make the behavior configurable by a cci_param to disable side effects

added enable_side_effect cci_param with default value true

@eyck can you please rereview the changes?

@eyck eyck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@eyck eyck merged commit ca0a38c into Minres:main Jun 5, 2026
4 checks passed
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