BIND 10 #2185: RR type implementation: TLSA

[BIND 10 Development]

#2185: RR type implementation: TLSA
-------------------------------------+-------------------------------------
            Reporter:  shane         |                        Owner:
                Type:  enhancement   |  pselkirk
            Priority:  high          |                       Status:
           Component:  libdns++      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  bind10-1.2-release-freeze
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by muks):

 * owner:  muks => pselkirk


Comment:

 Hi Paul

 Replying to [comment:10 pselkirk]:
 > My intent wasn't to cause more work, just to point out that
 {{{auto_ptr}}} has been deprecated, so this code might not work with some
 future version of the compiler.

 Nod. We have generally avoided using `std::auto_ptr` except where
 absolutely necessary, using `boost::scoped_ptr` and `boost::shared_ptr`
 instead as applicable.

 > The new {{{RdataPimplHolder}}} class is an interesting experiment, and
 probably not heavier-weight than {{{auto_ptr}}}, but it's more code to
 test and maintain (and to update the 7 other {{{Rdata}}} classes that use
 {{{auto_ptr}}}), so I would tend to resist it on those grounds.

 The `RdataPimplHolder` is used as drop-in replacement for `std::auto_ptr`,
 so it won't be difficult to understand it. The code and tests are written,
 and for something this straightforward and independent, it is unlikely any
 maintenance will be necessary.

 This whole exercise is because `std::auto_ptr` is deprecated. We should
 avoid using it in new code, and it's unlikely we can rely on C++11-only
 features like `std::unique_ptr`:

 The RDATA classes are a major user of `std::auto_ptr`. The rest of the
 classes also seem to use it similar to how the RDATA classes use it. In
 any other cases that do not require `.release()`, they use
 `boost::scoped_ptr` and `boost::shared_ptr`.

 I suggest even renaming this class and moving it to `libb10-util` as a
 general smart pointer.

 But if you have a strong opinion that it should not go in, I'll revert it
 and switch back to `std::auto_ptr`.

 > Also, it doesn't compile:
 > {{{
 > In file included from rdata_pimpl_holder_unittest.cc:15:0:
 > ../../../../src/lib/dns/rdata_pimpl_holder.h:27:31: error: ‘NULL’ was
 not declared in this scope
 >      RdataPimplHolder(T* obj = NULL) :
 >                                ^
 > ../../../../src/lib/dns/rdata_pimpl_holder.h:35:25: error: ‘NULL’ was
 not declared in this scope
 >      void reset(T* obj = NULL) {
 >                          ^
 > ../../../../src/lib/dns/rdata_pimpl_holder.h: In member function ‘T*
 isc::dns::rdata::RdataPimplHolder<T>::release()’:
 > ../../../../src/lib/dns/rdata_pimpl_holder.h:46:16: error: ‘NULL’ was
 not declared in this scope
 >          obj_ = NULL;
 >                 ^
 > rdata_pimpl_holder_unittest.cc: In member function ‘virtual void
 {anonymous}::RdataPimplHolderTest_all_Test::TestBody()’:
 > rdata_pimpl_holder_unittest.cc:39:19: error: call to ‘void
 isc::dns::rdata::RdataPimplHolder<T>::reset(T*) [with T = int]’ uses the
 default argument for parameter 1, which is not yet defined
 >      holder2.reset();
 > }}}

 It did not fail compile for me on Fedora 20 with GCC. I am guessing it is
 failing in a different build environment (perhaps with a different
 standard C++ library).

 I have now explicitly included `<cstddef>` that provides `NULL`'s
 definition.

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