Re: [dbus-cplusplus-devel] contributing a submission to the project...best way?

[Freyensee, J. P <jam...@in...>]

Andreas:

Was the feedback I gave in the last thread okay with you? Anything I need to do for you?  Were the remaining changes accepted?  

Thanks!
Jay


----------------------------------------------------------------------

Message: 1
Date: Fri, 20 Nov 2009 09:58:38 -0800
From: "Freyensee, James P" <jam...@in...>
Subject: Re: [dbus-cplusplus-devel] contributing a submission to the
	project...best way?
To: "dbu...@li..."
	<dbu...@li...>
Cc: "Yokoyama, Caz" <caz...@in...>
Message-ID:
	<A26...@or...>
	
Content-Type: text/plain; charset="us-ascii"

Andreas:

Thanks for your work.  It is very much appreciated.

On the examples, I don't think they are important.  I know changes were made just to get them to compile so if the examples compile for you, then that's good :-).


On:
>connection.h/connection.cpp:
>extern DXXAPI Dispatcher* default_dispatcher;
>...and all changes specific to the change that Connection constructors
>have a new parameter Dispatcher* dispatcher.

>I've a raw idea what's the reason for it. But could you explain what
>was the main idea behind this change.

I cannot give you an exact reason, but I can give you a general reason.  We want a safeguard of being able to set up a single default dispatch mechanism for all Dbus connections we try to establish in a project.  For example, there is this project we have open-sourced for a 3rd party:

https://sourceforge.net/projects/optelephony

that heavily uses this concept when trying to establish an initial session with the 3rd party's product:

// from https://sourceforge.net/projects/optelpehony
bool TelSessionClient::initSessionClient(){

   if(!mDispatcher){
    mDispatcher = new DBus::Glib::BusDispatcher();
    DBus::default_dispatcher = mDispatcher;
    mDispatcher->attach(NULL);
   }

   if(!mConnection){
    try{
    mConnection = new DBus::Connection(DBus::Connection::SystemBus());
    }
    catch (DBus::Error e) {
     printf("Can not connect to the SystemBus : %s\n",e.message());
     return 0;
    }
    mConnection->set_timeout(120*1000);
   }

   if(!cb)
    cb = new TelCallBack(*mConnection);

   if(!server)
    try{
     server = new TelServerTAPI (*mConnection);
    }
    catch (DBus::Error e){
     return false;
    }
   mDispatcher->enter();
   return true;
}

On:
>generate_adaptor.cpp/generate_proxy.cpp/generator_utils.cpp:

>What are exactly the reason of these change? Looks for me like some sort
>of Async method support. Could you spend some more words here?

that actually did not come from the change we wanted to apply to your project.  What looks like has happened is these files used to be in xml2cpp.cpp and were separated out at some point.  I believe the commit ID on the git tree when this happened was c2b8cb (maybe??).  I don't think this is really a part of the libdbus-c++ functionality as is a part of a tool to create .cpp code from .xml code, so I don't believe this effects us at all.

On:

> eventloop.cpp:
> DefaultMutex::DefaultMutex()
> {

>   pthread_mutex_t recmutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
>  _mutex = recmutex;
> //_mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
> //pthread_mutex_init(&_mutex, NULL);
>}

We don't seem to use the class DefaultMutex or the methods where the DefaultMutex instance variable _mutex_p is used (queue_connection(), dispatch_pending()).  And I know we did not comment that code out.  I could tell you a possible reason for why PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP is there.  It prevents a certain deadlock case if PTHREAD_RECURSIVE_MUTEX_INITIALIZER was used instead.  If PTHREAD_RECURSIVE_MUTEX_INITIALIZER was used, multiple calls of pthread_mutex_lock without calling pthread_mutex_unlock by the owner of the mutex could result in dead-lock.  I have no idea if the original intent was to be able to use this code recursively, but it may be a good idea to leave this alone (well, with the exception of removing the commented-out lines)?


I think I covered all of your concerns??  Please let me know if I did not.  Also, please let me know when the final changes are made.  I'm going to pull down the final result and run it with our project.

thanks again,
Jay

------------------------------