BIND 10 #2095: Define and implement RdataEncoder class

BIND 10 Development do-not-reply at isc.org
Mon Jul 30 20:06:31 UTC 2012


#2095: Define and implement RdataEncoder class
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120731
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  scalable inmemory                  |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:12 vorner]:

 Thanks for the review.

 > It doesn't compile for me. I think I saw this somewhere and it is maybe
 because you based it on another branch and it should be fixed in master,
 but a merge with master has conflicts:

 Ah, right.  I've cherry-picked the corresponding fix.

 > Some minor comments:
 >  * The exception text „RDATA encoder finds missing field„ looks slightly
 strange to me. If it is missing, then it can't be found.

 Tried to clarify it.  Is it okay now?

 >  * The code in `updateOtherData` is confusing. I'm not really sure how
 to improve that, maybe placing the variable length thing into an else
 branch instead of using continue could be an option.

 I agree it's not very understandable.  In fact I didn't really like
 that implementation either.  On closer look I found it could be much
 more simplified, and I did it at commit 8560883.

 > Also, I'd like to see the thing does not go outside the buffer that is
 passed to it. Would it be possible to do a test with a canary vale after
 the data and see it survived?

 you mean as a result of encode()?  I think the assert() check would
 catch any such bugs, but I've added some explicit check in the
 unittests with canary data (commit 84e7463).

-- 
Ticket URL: <http://bind10.isc.org/ticket/2095#comment:13>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list