BIND 10 #1784: b10-resolver incorrectly uses SyncUDPServer

[BIND 10 Development]

#1784: b10-resolver incorrectly uses SyncUDPServer
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20120320
                   Priority:  very   |            Resolution:
  high                               |             Sensitive:  0
                  Component:         |           Sub-Project:  DNS
  resolver                           |  Estimated Difficulty:  0
                   Keywords:         |           Total Hours:  0
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:5 jelte]:

 Thanks for the prompt review.

 > my compiler is apparently not as smart as yours, and it needs the non-
 const static vars in dns_service_unittest to be defined outside of the
 class declaration.
 >
 > cppcheck noted that data[] wasn't initialized, and, although it doesn't
 really matter what's in it, to make cppcheck happy, i just filled it with
 1's. And another thing it caught is that in getSocketFD, res was
 dereferenced before its NULL check, so i've moved the code there around a
 bit.
 >
 > Oh, and I got a linker error when linking against lib/libresolver,
 needed an LIBADD for that.
 >
 > I have taken the liberty to address these issues myself, please check in
 commit

 Thanks for the checks and corrections.  These seem good.  I've added
 comments about the null check after getaddrinfo() because, technically
 (per the API contract) the check should be redundant.

 > So the addServer calls are the thing you were wondering we should
   keep? I'd say remove them. (they even use the old dlog() calls, but
   have other problems as well)

 Yeah I think we should remove them, even if we have vague expectation
 that we may want to reuse them in future (YAGNI applies here).  I
 think it should go to a separate ticket (and guess it would be a
 perfect item for "hardening").

 > Should we warn/error on unknown options in addServerUDPFromFD?
 (currently they are ignored)

 Hmm, probably.  Although it's quite unlikely to be misused (because it's
 enum and you need to use a cast to decieve the compiler), it could still
 be possible that an application built with a new version of header
 (and using a new option flag) uses an old version of library binary.

 So I've added the check.

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