BIND 10 #3054: Add library validation to the hooks framework

[BIND 10 Development]

#3054: Add library validation to the hooks framework
-------------------------------------+-------------------------------------
            Reporter:  stephen       |                        Owner:
                Type:  enhancement   |  stephen
            Priority:  medium        |                       Status:
           Component:  Unclassified  |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130731
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  0             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tomek):

 * owner:  tomek => stephen


Comment:

 Replying to [comment:5 stephen]:
 > > It would be better to pass unsigned int and then no exception would be
 needed at all.
 > I had thought of that, but "ints" are used in the code and in some cases
 negative values have meaning.  I'm wary about mixing signed and unsigned
 values as it is all to easy to write code in which a negative number is
 promoted to an unsigned int without warning.
 Ok.

 > Currently, the only use case for a list of libraries that fail to
 validate is to include it the answer sent back to bindctl.  That requires
 the elements of the list to be concatenated into a single string.
 However, on reflection your comment is valid - the set of failing
 libraries should be returned in some form of structure (vector) and it is
 the function that receives that data that should format the strings into
 an error message.  The code has been changed.
 Thank you. The new code looks fine.

 > > General comment: We had a discussion about memory allocated in the
 libraries being freed after the library is unloaded. One of the issues was
 a !CalloutHandle object for existing libraries. There are 2 places that
 store the !CalloutHandle: Dhcpv6Srv::getCalloutHandle(const Pkt6Ptr& pkt)
 and Dhcpv4Srv::getCalloutHandle(const Pkt4Ptr& pkt). I think both should
 be extended to release current !CalloutHandle object when passed with a
 NULL Pkt{4,6}Ptr. I see that the master 3054 was branched from is a bit
 old and doesn't include DHCPv6 code (which was merged couple days ago). I
 think a new ticket should be created to point out that
 getCalloutHandle(NULL) should be called before unloading old libraries.
 > I was planning to do that in #2981 but have created #3062 for it.  I'll
 need to complete that ticket before I complete #2981.
 Ok. Additional tickets is the preferred way to go.

 > > I hope that the documentation ticket will include an overview of all
 the classes. I must admit that it is still a bit confusing. I was thinking
 about something like we have for configuration parsers (see Developer's
 guide -> DHCPv6 server component -> Configuration parsers in DHCPv6). It
 doesn't have to be long, just give brief overview with pointer to detailed
 class descriptions.
 > I was going to do this as part of #2938, but instead have created #3063
 for the Doxygen form of the documentation.  I'll still update the wiki
 design document and will close #2938 at that point.
 Ok.

 That addresses all my comments. Please merge. Make sure that you update
 ChangeLog. The text you proposed in comment:2 is fine.

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