Skip to content
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

#420 breaks the mesh reply list function that need to reuse the dns answer. #517

Merged
merged 1 commit into from Aug 3, 2021

Conversation

dyunwei
Copy link
Contributor

@dyunwei dyunwei commented Aug 3, 2021

Sorry for my previous change causes another issue.

My previous change that clears the c->buffer in the comm_point_send_reply does resolve the "can't fit qbuffer in c->buffer" issue, but it breaks the mesh reply list function that need to reuse the answer. Since the c->buffer is cleared in the comm_point_send_reply, it cannot be resued again. it means that it is not inappropriate to clear c->buffer in the comm_point_send_reply.

After some investigation, i found it is appropriate to clear c->buffer before use in the http2_query_read_done.

clear the c->buffer in the comm_point_send_reply does resolve the "can't fit qbuffer in c->buffer" issue, but it breaks the mesh reply list function that need to reuse the answer. because the c->buffer is cleared in the comm_point_send_reply, it cannot be resued again. it means that it is not inappropriate to clear c->buffer in the comm_point_send_reply.

After some investigation, i found it is appropriate to clear c->buffer before use in the http2_query_read_done.
@dyunwei dyunwei changed the title #420 #420 breaks the mesh reply list function that need to reuse the dns answer. Aug 3, 2021
@wcawijngaards wcawijngaards merged commit 5196ee0 into NLnetLabs:master Aug 3, 2021
wcawijngaards added a commit that referenced this pull request Aug 3, 2021
- Merge PR #517 from dyunwei: #420 breaks the mesh reply list
  function that need to reuse the dns answer.
@wcawijngaards
Copy link
Member

Thanks for the fixup, merged it into the code repository.

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.

None yet

2 participants