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:  jelte
                       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      |
-------------------------------------+-------------------------------------

Comment (by 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).

 Replying to [comment:5 jelte]:

 > > - I don't see the point of (or need for) the changes in
 > >   messagerenderer_python.h.  Same for rcode_python.h.
 >
 > It may be trivial, but using the declaration from the header instead of
 a
 > forward declaration seemed more clean to me.

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

 In this particular case, since the changes are not relevant to this
 task I'd personally suggest keeping the original code, but I don't
 strongly insist on it.  I'd leave it to your discretion.

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

 But I'd leave this point to you, too.

 Other parts of the code (and the response comments to the review) look
 okay.

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