11:19 p.m.
On Fri, Aug 5, 2011 at 12:45 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
On Fr, 2011-08-05 at 11:03 +0300, Dumez, Christophe wrote:
> + if (!e_book_client_get_view_sync(m_addressbook.get(), sexp,
> &view, NULL, gerror)) {
> + gerror.throwError( "getting the view" );
> + }
> + EBookClientViewCXX viewWrapper = EBookClientViewCXX::steal(view);
Would it make sense to extend SE_GOBJECT_TYPE = EBookClientViewCXX such
that it supports the same initialization as GListCXX by simply passing
it to GNOME functions?
Like this:
EBookClientViewCXX view;
if (!e_book_client_get_view_sync(m_addressbook.get(), sexp, view, NULL,
gerror)) {
gerror.throwError( "getting the view" );
}
Implementing it won't be as easy as for GListCXX, because
EBookClientViewCXX is a boost::shared_pointer. Just wondering... ;-)
Good idea, I implemented that and fixed the code.
+ EvolutionContactAsyncList async_list(view);
+
+ if (!async_list.m_error.isNull()) {
+ async_list.m_error.throwError("watching view");
+ }
+
+ GSList *nextItem = async_list.m_contacts;
+ GListCXX<EContact, GSList, GObjectDestructor<EContact> >
listptr(nextItem);
Indention problem, but my main question is: how is ownership of the list
elements handled here? Isn't there a double unref, once inside
async_list.m_contacts and once in listptr?
I fixed the indentation problem and there was indeed a double-unref
that is fixed now.
From GLibSupport.h:
+template<class T> void GObjectDestructor(T *obj);
Does that really work? Doesn't the template definition have to be
visible here for it to be used?
Well, it compiled at least. I moved the implementation to the header
file to be safe.
+/**
+ * Wraps a C char array and takes care of freeing the memory.
+ */
+class PlainCStr : public boost::shared_ptr<char>
+{
+ public:
+ PlainCStr() {}
+ PlainCStr(char *str) : boost::shared_ptr<char>(str, free) {}
+ PlainCStr(const PlainCStr &other) :
boost::shared_ptr<char>(other) {}
+ operator const char *() const { return &**this; }
+ const char *c_str() const { return &**this; }
+};
Note that free() != g_free(). This PlainCStr is used for strings
allocated by GNOME, isn't it? Then we need a second PlainGStr which uses
g_free().
Replaced char / free() by gchar / g_free() and renamed to PlainGStr.
Other than that the patch looks good. Does it pass the
Client::Source::eds_contact tests?
Yes it does:
$ CLIENT_TEST_SOURCES=eds_contact CLIENT_TEST_EVOLUTION_PREFIX=Test_
client-test Client::Source::eds_contact
Client::Source::eds_contact::testOpen okay
Client::Source::eds_contact::testIterateTwice okay
Client::Source::eds_contact::testSimpleInsert okay
Client::Source::eds_contact::testLocalDeleteAll okay
Client::Source::eds_contact::testComplexInsert okay
Client::Source::eds_contact::testLocalUpdate okay
Client::Source::eds_contact::testChanges okay
Client::Source::eds_contact::testImport okay
Client::Source::eds_contact::testImportDelete okay
Client::Source::eds_contact::testManyChanges okay
OK (10)
Remark: I also had to extend SE_GOBJECT_TYPE() macro to support a
"base" parameter as certain GObjects such as EBookQuery have their own
ref/unref functions. SE_GOBJECT_TYPE(_x) still behaves the same way
but it now calls internally the new SE_GOBJECT_BASED_TYPE(_x, base)
macro, using GObject as base.
Kr,
--
Dr. Christophe Dumez
Linux Software Engineer
Intel Finland Oy - Open Source Technology Center