On Fri, 2012-03-16 at 13:11 +0100, Krzesimir Nowak wrote:
> 2012/3/15 Patrick Ohly <[email protected]>:
> > On Thu, 2012-03-15 at 10:34 +0100, Krzesimir Nowak wrote:
> >> 2012/3/14 Patrick Ohly <[email protected]>:
> >> > Hello Chris!
> >> >
> >> > You added a callback parameter to addResource() (formerly known as
> >> > enqueue()) and related functions. There's no documentation for that in
> >> > server.h/cpp. What is it meant to be used for, what kind of constraints
> >> > does it have, etc.? In other words, please document it for me... ;-)
> >> >
> >>
> >> That is my doing again. Will document it. The callback was added,
> >> because addResource calls checkQueue() which calls SetActive, which is
> >> asynchronous. The callback is called when after all SetActive calls
> >> are finished or, if there was no call to SetActive, at the end of
> >> checkQueue(). In short - addResource, removeResource are asynchronous
> >> too, so given callback is called when they are finished.
> >
> > Instead of just calling the parameter "callback" it would be good to use
> > a name which is related to the time when the callback is invoked.
>
> Well, maybe - the callback is called after in fact adding or removing
> resource is finished. I have no idea what other name it should have. I
> will have to just document it when it may be called.
In SessionResource for createSessionResource(), init(), and
onSessionConnect() a better name would be "constructionCompleted". Note
my previous comments about the lack of a "constructionFailed".
In Server, in checkQueue() and addResource() use the same kind of
callback, but I don't understand what kind of guarantee is made about
it. Something like "activatedOrFailed"?
It's not the same as SessionResource::setActiveAsyncCb's "callback",
although it has the same name and signature.
> > The code is here:
> > // If not we activate the resource and place it in the active
> > resource list.
> > if (canRun) {
> > m_activeResources.push_back(waitingResource);
> > // If this is a session, we set active and emit
> > sessionChanged to clients.
> > boost::shared_ptr<SessionResource> session =
> >
> > boost::dynamic_pointer_cast<SessionResource>(wq_iter->lock());
> > if (session) {
> > ++(*counter);
> > session->setActiveAsync(true,
> > boost::bind(&Server::setActiveCb, this, session, counter, callback));
> > }
> > m_waitingResources.erase(wq_iter++);
> > }
> >
> > That seems unnecessarily complex to me, but perhaps I am missing something.
>
> That was the case that if two sessions are going to be active then we
> don't want to have our callback called twice.
If I knew what the callback stands for, then perhaps I would understand
the code :-/
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
_______________________________________________
SyncEvolution mailing list
[email protected]
http://lists.syncevolution.org/listinfo/syncevolution
