On Thu, Sep 23, 2010 at 2:20 PM, Branko Čibej <br...@xbc.nu> wrote: > On 22.09.2010 21:41, Hyrum K. Wright wrote: >> [ apologizes for the somewhat stream-of-conscious nature of these mails ] >> >> On Wed, Sep 22, 2010 at 7:16 PM, Hyrum K. Wright >> <hyrum_wri...@mail.utexas.edu> wrote: >>> On Wed, Sep 22, 2010 at 5:35 PM, Hyrum K. Wright >>> <hyrum_wri...@mail.utexas.edu> wrote: >>>> For the C++ folks out there, I've got a question about an approach to >>>> take on the object-model branch. At issue is how to wrap the various >>>> C structures returned to callers, particularly in a backward >>>> compatible manner. Currently, I'm looking at svn_wc_notify_t *. As I >>>> see it, there are a few options: >>>> >>>> 1) Just wrap the pointer to the C struct as a member of the wrapper class. >>>> Pros: Easy to implement; lightweight constructor. >>>> Cons: Getters would need to translate to C++ types; would need to >>>> implement a copy constructor which deep copies the C struct; would >>>> also introduce pools, since creating and duplicating C structs >>>> requires them. >>>> >>>> 2) Wrap each C struct member individually >>>> Pros: C->C++ complexity is constrained to the constructor, >>>> everything else is C++ types >>>> Cons: Hard to extend for future compatibility >>>> >>>> 3) Just pass the C-struct pointer around; don't even bother with a class >>>> Pros: Dead simple. >>>> Cons: Requires more memory management thought by consumers; not >>>> C++-y enough; may introduce wrapping difficulties. >>>> >>>> I'd like to come up with something consistent, which would be used >>>> throughout the C++ bindings. I'm also interested in a solution which >>>> ensures the C++ bindings can be used as the basis for other >>>> object-oriented bindings models (Python, Perl, etc.) >>> After lunch, and some thought, it feels like #1 is the best solution. >>> This doesn't change the external class interface, which is good, and >>> can still provide C++ values to callers who want them. The pool >>> issues are a bit messy, but at least the object can manage it's own >>> memory (albeit at a significant overhead). >> This could get ugly. >> >> Creating and destroying pools all over the place could get ugly, but >> it's necessary evil because all of our object creation / duplication >> functions all require a pool. An alternative would be a set of >> functions returning the size of the object, and then another which >> puts the object in a pre-allocated memory location. (These could >> theoretically replace the pool argument version of the API, but that'd >> be *a lot* of churn.) >> >> The approach would let the C++ allocate the memory (of the correct >> size) using whatever scheme it wants, and then do a "placement >> initialize" using the second API. If we do go this route, I'd >> recommend exposing these as private-to-Subversion, at least initially. >> >> The other option is just pass the C-struct pointer around everywhere, >> but then the bindings consumers have to work about this exact same >> issue. In other words, it solves it now, but really just pushes the >> problem elsewhere. >> >> -Hyrum > > Memory management with pools and C++ -- keep away from doing it in > per-object ctor/dtor pairs is all I can say. Lifetimes would get so > messy and mucked up you could hardly believe it. > > IMHO the best way to wrap the C structures in C++ is to subclass 'em. > > class svn_some_thing : public svn_some_thing_t { .... }; > > This way you can pass C++ pointers directly to the C implementation, > most methods become just inlined wrappers. Callbacks are a bit more > hairy, but not all that much.
This works for input types, but right now it's the output types (which are much more common) that I'm concerned about. Things like svn_commit_info_t, or svn_wc_notify_t. For my initial hack at it see: http://svn.apache.org/repos/asf/subversion/branches/object-model/subversion/bindings/c++/include/Types.h?p=1000600 > As to pools ... I'd once thought that a proper C++ wrapper for a pool > would be a reference-counted smart pointer. That turns out to be too > much overhead and too much of a good thing, since you *cannot* allow > pool lifetime to depend on some reference counting order of execution. > Best just use plain pool pointers, or maybe wrap them minimally so that > destructors do the pool cleanup reliably. Yeah, we've got a Pool class which destroys the underlying APR pool in the dtor, so that provides a handy way to clean things up (if it weren't for the 8k overhead of the minimum pool size). To the initial hack, I've added a smart pointer to prevent multiple redundant copies of the underlying C struct, since I expect it to be read-only (we could probably add an explicit Clone() method if folks really wanted a deep copy). The smart pointer magic is here: http://svn.apache.org/repos/asf/subversion/branches/object-model/subversion/bindings/c++/include/Types.h There's still a self-managed pool for allocating the C-struct, but it's encapsulated enough to be changable should we have a better way of duplicating objects in the future. Thanks for the insight, by the way. These classes and paradigms are in no way set in stone, and I welcome discussion. I sense you've got a bit more C++ design experience than I do. :) -Hyrum