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:  reviewing
                       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      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:6 jinmei]:
 > -  Rdata_toWire() seems to be a bit buggy: I suspect
 >    PyBytes_FromStringAndSize() can return NULL, in which case a
 >    disaster will happen.

 As that would mean a python exception, with it already set, I just return
 NULL.

 > - we should be able to eliminate reinterpret_cast.

 Done. Actually, when I was doing this, I opened the wrong file by
 accident. When I noticed, it was already done, so I left it there. But if
 you think it is too much for the ticket, I could remove these changes.

 > - I'd add more complete documentation (more aligned with the original
 >    C++ doxygen doc)

 Hmm, the docs don't seem even remotely related to the purpose of the task,
 and it doesn't seem as problematic as uncaught exceptions. So I'd leave
 them out.

 For the exceptions, I added a wrapper function. Maybe it could be made
 generic and handle even the pointer casts for the self and be put into
 some kind of utils library?

 I tried to make the function to it a template parameter, so I wouldn't
 need to write the internal and external function explicitly, but the
 compiler didn't seem to agree with me, so it is done explicitly. It could
 be done with a macro, but I know we don't like these.

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

 So, something like:
 {{{
 [bug]           vorner
 C++ exceptions in the isc.dns.Rdata wrapper are now converted
 to python ones instead of just aborting the interpretter.
 }}}

 > - 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.

 Hmm, it makes some sense. I was thinking about catch as a separate
 statement, but it is more like an else to if, so it belongs together.
 Done.

 > - the input for the CharStringTooLong case in the test could be more

 Thanks, I didn't know I can do this in python as well. It surprises me, if
 it doesn't have ++, but can multiply strings O:-).

 Thank you

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