BIND 10 #1407: isc.dns.Rdata should catch C++ exceptions

[BIND 10 Development]

#1407: isc.dns.Rdata should catch C++ exceptions
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  accepted
                       Type:         |             Milestone:
  defect                             |  Sprint-20111206
                   Priority:         |            Resolution:
  critical                           |             Sensitive:  0
                  Component:         |           Sub-Project:  DNS
  libdns++                           |  Estimated Difficulty:  2
                   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:4 vorner]:
 > Hello
 >
 > I made it catch whatever I thought off and added the fallback to the
 _init function. I didn't add any catches to the other methods, they are
 far less likely to throw. Should they be added?

 Since the original task is quite small, I'd fix them using this
 opportunity.  I'd even fix some other minor bugs and make other
 cleanups:

 -  Rdata_toWire() seems to be a bit buggy: I suspect
    PyBytes_FromStringAndSize() can return NULL, in which case a
    disaster will happen.
 - we should be able to eliminate reinterpret_cast.
 - I'd add more complete documentation (more aligned with the original
    C++ doxygen doc)

 This wrapper was written at an earlier stage of development in a rush,
 and was generally not very clean.  By piggybacking cleanups with a
 relatively bigger bug fixes we can incrementally improve the quality
 of the code.

 But on the other hand these may be an abuse of the ticket especially
 because we gave a small point of estimation to this ticket.  So I'd
 leave the decision to you.

 > And, should this have a changelog entry?

 I think so, because it's a bug in a public API.

 The change in the branch basically looks okay.  I have just two minor
 comments:

 - To be consistent with most of other code of the source tree I'd
   place 'catch' at the same line as that for the closing brace of the
   previous clause:
 {{{#!c++
     } catch (const isc::dns::rdata::InvalidRdataText& irdt) {
         PyErr_SetString(po_InvalidRdataText, irdt.what());
         return (-1);
     }
     ...
 }}}
   but this may purely be a matter of taste.
 - the input for the CharStringTooLong case in the test could be more
   simplified:
 {{{
         self.assertRaises(CharStringTooLong, Rdata, RRType("TXT"),
                           RRClass("IN"), ' ' * 256)
 }}}
   (unless you intentionally generated 's' this way for some particular
   reason)

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