BIND 10 #1245: pydnspp as dynamic module (so other wrappers can use it)

[BIND 10 Development]

#1245: pydnspp as dynamic module (so other wrappers can use it)
-------------------------------------+-------------------------------------
                   Reporter:  jelte  |                 Owner:  jinmei
                       Type:  task   |                Status:  reviewing
                   Priority:  major  |             Milestone:
                  Component:         |  Sprint-20110927
  libdns++                           |            Resolution:
                   Keywords:         |             Sensitive:  0
            Defect Severity:  N/A    |           Sub-Project:  DNS
Feature Depending on Ticket:         |  Estimated Difficulty:  0
        Add Hours to Ticket:  0      |           Total Hours:  0
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => jinmei


Comment:

 Replying to [comment:13 jinmei]:
 > About the revised code:
 >
 > - Message_getEDNS() needs to catch exceptions from createEDNSObject().
 > - same for Message_getQuestion(), Message_getSection(),
 >   Question_getXXX(), Question_toText() (although it's not due to this
 >    branch), RRset_getXXX(), RRset_getRdata() (and perhpas more)
 > - createMessageRendererObject() is not exception safe.  I'm not sure
 >   if it's okay to make a copy either.  Since it doesn't seem to be
 >   used for now, maybe we should rather keep it undefined for now
 >   (possibly with comments about the open issues).
 >

 added try-catch around all calls to createXXXObject. Also added an extra
 return
 value check in one of the list builders in message.h

 Removed the createMessageRenderer() function (btw i think this was the
 most
 sane approach. Well, apart from the exception-safety of course :))

 >
 > Actually, I intentionally avoided including header files (I guess I
 > originally wrote this part of the code) because importing a full
 > header could expose a bunch of unnecessary definitions with the
 > possible associated overhead, often recursively (via other header
 > files that this header file internally includes), thus possibly
 > contributing to longer compile time, making the code fragile in terms
 > of portability, etc.
 >
 > On the other hand I admit manually introducing forward declarations
 > don't look cool either.  Ideally we might separate "forward
 > declarations headers" and others, but we don't do it (or we may not
 > agree on whether it's a good idea project-wide).
 >

 Hmm. I'd say this is less portable :) Until one of the more recent changes
 (mainly the move of initModulePart to pydnspp.cc), whatever included
 xxx_python.h tended to include xxx.h anyway. BTW I am also of the opinion
 that
 we tend to split things up too much sometimes :)

 But anyway, i've reverted to forward declarations where possible.
 pydnspp.cc now
 needs a few extra includes (no biggies). I've left in two cases of header
 includes (rrset and rdata), where there are xxxPtr types in the API (for
 compiler safety i'd prefer those defines to be done only once).

 >
 > > > - PyName_Check, PyName_ToName: should we check and reject NULL
 > > >   pointers more explicitly?
 > >
 > > Hmm. [...]
 > > specifically skipped this check because *most* of the uses are
 directly after
 > > ParseTuple("O!"), in which case the type is guaranteed.
 > >
 > > Note that we could also choose to return false if the argument to
 _Check() is
 > > NULL. However, IMO it really is a programming error, and should raise
 > > (returning false might hide problems).
 >
 > If we should be able to expect it always be non NULL, I'd even pass it
 > by reference than by pointer, shifting the responsibility of check (if
 > needed at all) or the consequence of a bug to the caller side.
 >

 Okay okay, I've added NULL checks :) Raises a Wrapper exception if null.

 I prefer to keep the argument a raw pointer since that is what's used in
 the
 python C/API, and nearly every function that involves them either takes or
 returns it.

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