After looking at the lock handling that is used for user and owner
objects (and possible other daemon resources as well) I've seen that the
DropUser() calls appear everywhere cluttering up the code and are easy
to forget when coding.
I think it would be preferable to use guard classes. (I don't know if
this is the correct term but it's what they're called at work for our
mutex handling.)
Instead calling FetchUser() and getting a pointer back which must
returned via DropUser() a guard object is created which holds the
pointer and the guard destructor automatically calls DropUser() when the
guard goes out of scope.
Example old code:
ICQUser *u = gUserManager.FetchUser(szId, nPPID, LOCK_R);
int messages = u->NewMessages();
gUserManager.DropUser(u);
Example of new code:
// If the current scope is too long, add braces to limit the scope
{
// Create a guard object, the constructor will call
// gUserManager.FetchUser() and store the returned ICQUser pointer
// internally in the guard.
IcqUserGuard u(szId, nPPID, LOCK_R);
// No difference here. Operator -> is overloaded to make the guard
// transparent.
int messages = u->NewMessages();
// When the Guard goes out of scope, the destructor is called which will
// do a DropUser() on the ICQUser.
}
The guard destructor will always be called. No need to call DropUser()
before returns, breaks or in else-blocks. Also exceptions can be thrown
anywhere.
It should be simple to implement as the guard will require very little
code to work. New code can use the guard while existing code will be
unaffected and can be kept as is until someone takes to time to do the
search and replace.
I'm not sure if this is something to add to the current daemon or if it
should be part of the new API only, but my vote is on the current code
as it is not much work but makes lock handling so much easier.
/Anders
