BIND 10 #2591: DHCPv4 server to include configured options in its responses to a client

[BIND 10 Development]

#2591: DHCPv4 server to include configured options in its responses to a client
-------------------------------------+-------------------------------------
            Reporter:  marcin        |                        Owner:
                Type:  task          |  marcin
            Priority:  high          |                       Status:
           Component:  dhcp4         |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130122
         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 => marcin


Comment:

 Replying to [comment:5 marcin]:
 > Replying to [comment:4 tomek]:
 > > Review of all changes present on
 933430e40bb35135d741df5b9c8b944c5bf9be83
 > > '''src/bin/dhcp4/dhcp4_srv.cc'''
 > > Please remove HARDCODED_DNS_SERVER, HARDCODED_DOMAIN_NAME.
 > Updated.
 The only hardcoded thing on 2591 branch is HARDCODED_SERVER_ID.
 Fortunately, it is removed on master already. It seems we are good to go
 here.

 > > '''appendRequestedOptions''':
 > > The code aborts if there was nothing requested (no PRL). Wouldn't it
 be more helpful to assign some basic (netmask, gateway, domain, dns)
 options to buggy clients that didn't send PRL? This is an honest question.
 It's a tradeoff between strict RFC compliance and support for
 clueless/buggy clients.
 >
 > I added the function that appends basic options if they are not
 requested but configured.
 Great, easy to understand and extend method. I like it.

 > > '''appendDefaultOptions'''
 > > I think this method should insert all options that have persistent
 flag set.
 > The persistency flag is not used/configured yet.
 Ack.

 > > I would prefer if OptionIntArray template class had method addCode()
 or addValue(). It would be easier to manipulate and more efficient (no
 need to create temporary vector that is later copied).
 > I added a new function and the unit test.
 It's easier to read now. Thanks. There's unused opts vector defined in
 line 96.

 > > Please update ChangeLog.
 ChangeLog is almost ok, but please "b10-dhcp4:" at the beginning (or mark
 that is applies to v4 code in some other way). Please update commit-it
 once you merge it.

 > Please update BIND10 Guide: give examples on how to define options and
 remove HARDCODED_* list in section 16.2.
 Since the Guide on master changed significantly since you branched, I
 suggest you merge 2591 into master and open another ticket shortly
 afterwards for docs update.

 Ok, it seems that with those minor things (remove unused variable and
 ChangeLog updates), this is ready for merge.

 I don't need to see this ticket again.

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