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

RR type encoding length #33

Open
pusateri opened this issue Mar 28, 2018 · 4 comments
Open

RR type encoding length #33

pusateri opened this issue Mar 28, 2018 · 4 comments

Comments

@pusateri
Copy link
Contributor

Currently, the RR type encoding length is set to the length of the RR type data plus 2. RR type A has length 6, RR Type AAAA has length 18, etc. I think this includes the RDATA LEN short but it seems like it shouldn't.

@pusateri
Copy link
Contributor Author

If you agree this should be changed, I can submit a patch that changes all of the RR types to return the RDLEN and write the RDLEN in question.encode() and answer.encode() instead of in each RR type encoder.

@silverwind
Copy link
Collaborator

Yes, this seems a bit odd. Do we actually need these encodingLength properties? Why not just buf.length?

@pusateri
Copy link
Contributor Author

pusateri commented Apr 5, 2018

encode() uses them to calculate the buffer length to allocate:
L908
Which means it runs the same code twice for name and string encoding to calculate the length the first time and to copy into the buffer the second time. This seems like a lot of work to pre-calculate the length of the Buffer. Passing in a buffer to encode() gets around this issue and then none of the encodingLength code is used.

@pusateri
Copy link
Contributor Author

pusateri commented Apr 5, 2018

But this is secondary to the problem that the encoders for each RR type include the extra RDLEN bytes.

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

No branches or pull requests

2 participants