Skip to content

Add tests for reading a control stream #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 23, 2025
Merged

Conversation

tatsuhiro-t
Copy link
Member

No description provided.

@tatsuhiro-t tatsuhiro-t added this to the v1.10.0 milestone Apr 23, 2025
@tatsuhiro-t tatsuhiro-t requested a review from Copilot April 23, 2025 12:49
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds tests for reading a control stream by modifying the behavior in nghttp3_conn_read_control to return the number of bytes consumed immediately when the input buffer is exhausted.

  • Replaces a break with an early return when "p == end" in the frame length state.
  • Introduces a similar early return in the priority update state.
Comments suppressed due to low confidence (2)

lib/nghttp3_conn.c:664

  • Switching from 'break' to an early 'return' changes the control flow in the state machine. Ensure that all functions calling nghttp3_conn_read_control correctly handle this early return behavior and that no additional frame processing is expected after reaching the end of input.
return (nghttp3_ssize)nconsumed;

lib/nghttp3_conn.c:973

  • The early return in the priority update state mirrors the change above. Verify that this change is consistent with the state machine’s overall flow and that all caller logic is updated accordingly.
return (nghttp3_ssize)nconsumed;

@tatsuhiro-t tatsuhiro-t merged commit ba42bd2 into main Apr 23, 2025
36 checks passed
@tatsuhiro-t tatsuhiro-t deleted the add-read-control-tests branch April 23, 2025 12:55
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.

1 participant