BIND 10 #2276: avoid uninitialized variable from in-memory getClosestNSEC

BIND 10 Development do-not-reply at isc.org
Thu Sep 20 18:11:17 UTC 2012


#2276: avoid uninitialized variable from in-memory getClosestNSEC
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |                       Status:  new
            Priority:  medium        |                    Milestone:  Next-
           Component:  data source   |  Sprint-Proposed
           Sensitive:  0             |                     Keywords:
         Sub-Project:  DNS           |              Defect Severity:  N/A
Estimated Difficulty:  0             |  Feature Depending on Ticket:
         Total Hours:  0             |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
 valgrind reported use of some uninitialized variables:
 http://git.bind10.isc.org/~tester/builder//BIND10-valgrind/20120920071345-CentOS5-x86_64-GCC/logs/valgrind.out

 valgrind seems to be correct.  When we do things like:
 {{{#!cpp
             const ZoneNode* nsec_node;
             const RdataSet* nsec_rds = getClosestNSEC(zone_data,
                                                       node_path,
                                                       &nsec_node,
                                                       options);
 }}}
 nsec_node can be left uninitialized and subsequently used.

 A straightforward fix is to set it to NULL at declaration, but I'd
 rather solve it by making the interface safer, i.e., instead of
 passing `ZoneNode**` to getClosestNSEC(), let getClosestNSEC return a
 pair of `ZoneNode*` and `RdataSet*`.

 The attached patch implements the idea.

 If we don't immediately apply the change as an urgent care fix, I'd
 also like to propose some cleanups:

 - rename find_internal findInternal per naming convention
 - rename some of `rrset` variables to something like `rdset` or
   `rdataset`.   For example, rename the member variable of
   `FindNodeResult`:
 {{{#!cpp
     const RdataSet* rrset;
 }}}
    because naming an `RdataSet` `rrset` can be misleading.
 - change the order of the `rrset` (which should be named `rdataset`)
   and `node` parameters of createFindResult():
 {{{#!cpp
 createFindResult(const RRClass& rrclass,
                  const ZoneData& zone_data,
                  ZoneFinder::Result code,
                  const RdataSet* rrset,
                  const ZoneNode* node,
                  bool wild = false,
                  const Name* qname = NULL) {
 }}}
   because it's more consistent with other functions/methods that take
   both a node and rdataset, and because it would be the more natural
   order if we consider the pair of them as an `RRset`.

-- 
Ticket URL: <http://bind10.isc.org/ticket/2276>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list