Skip to content

Bugfix: negative return value from SerializeTo/DeserializeFrom#1690

Merged
vazois merged 1 commit into
microsoft:devfrom
chenhao-ye:chenhaoy/fix-parse
Apr 10, 2026
Merged

Bugfix: negative return value from SerializeTo/DeserializeFrom#1690
vazois merged 1 commit into
microsoft:devfrom
chenhao-ye:chenhaoy/fix-parse

Conversation

@chenhao-ye

Copy link
Copy Markdown
Contributor

Fix SerializeTo and DeserializeFrom: the return value should be the number of bytes (de)serialized and thus, non-negative.

The prior implementation misordered operands and returned a negative value.

Copilot AI review requested due to automatic review settings April 10, 2026 19:11

Copilot AI 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.

Pull request overview

Fixes incorrect (negative) byte-count return values from low-level (de)serialization helpers by correcting pointer subtraction order, ensuring callers receive a non-negative “bytes processed” result.

Changes:

  • Correct SessionParseState.SerializeTo to return curr - dest (bytes serialized).
  • Correct SessionParseState.DeserializeFrom to return curr - src (bytes deserialized).
  • Correct ObjectInput.DeserializeFrom to return curr - src (bytes deserialized).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
libs/server/Resp/Parser/SessionParseState.cs Fixes serialized/deserialized byte-count return values to be non-negative.
libs/server/InputHeader.cs Fixes ObjectInput.DeserializeFrom to report bytes consumed correctly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@vazois vazois merged commit 952de49 into microsoft:dev Apr 10, 2026
47 of 49 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.

3 participants