| < Previous by Date | Date Index | Next by Date > |
| < Previous in Thread | Thread Index | Next in Thread > |
Ses reply inlined.
On 3/31/07, Scott Godin <slgodin@xxxxxxxxxxxx> wrote:
> Hi Nash,
>
> Most of this looks good to me. I like the Command stuff you started -
> it should help users of dum to be thread safe.
>
> My only concerns are:
> 1. We need to comment in all header files - what the differences are
> between the xxxx and xxxxCommand methods are. Ie. one is asynchronous
> and thread safe.
> 2. Can you explain why you need the ability to pass a mutux to the
> DialogUsageManager process functions?
So, I offered a chance to synchronize 2 or more threads access (and
offered potential dead lock).
[Scott] huh? deadlock? You want to offer the ability to call process from multiple threads? That seems a little stange, since only thread can be in the process call at one time.
> 3. Looks like the ExternalMessage has some strange Win32 isms (ie. use
> of Win32ExportDum.hxx), that I'm not sure are appropriate for a general
> commit.
Actually the ExternalMessage is platform independent, we've made our
branche's DUM DLL exportable (resiprocate too), that is the usuage of
Win32ExportDum.hxx. Do I need to explain the usage of ExternalMessage?
[Scott] - Yes I realize that. It's just strange that the only thing in DUM that has this DUM_API stuff attached to it, is the ExternalMessage. Committing that code may only serve to confuse people.
> 4. I'm not a big fan of the onAckReceived callback you added - since it
> will end up getting called for session timer refreshes - if this is
> something that is required it should be designed a bit better. Also, it
> conflicts in naming with already existing onConnectedConfirmed callback.
There is difference between onConnectedConfirmed and onAckReceived, as
onConnectedConfirmed only get called once when dialog is setting up,
but on onAckReceived is triggered upon re-INVITE.
[Scott] It's confusing having 2 callbacks for ACKs with different names, and the names don't help to differentiate their use. Also, my other point here (about session timers) still applies.
>
> Anyone else?
>
> Scott
>