[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    Re: [Kde-pim] D-Pointer  problem
From:       Ingo =?iso-8859-15?q?Kl=F6cker?= <kloecker () kde ! org>
Date:       2008-05-31 21:44:56
Message-ID: 200805312345.04900 () erwin ! ingo-kloecker ! de
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


On Saturday 31 May 2008, Tom Albers wrote:
> Hi,
>
> I've added d-pointers for two classes in kpimidentities, but i can
> not get it to work properly. The stuff compiles ok, but as soon as i
> open the config from mailody it crashes in the identititymanager.
>
> If i revert the patch for the signature files, it won't crash. This
> is all with recompilation of mailody in between. I've spent some
> hours in trying to fix it, but I failed.
>
> Maybe someone with 'fresh' eyes, is willing to try the patch, see if
> it crashes too and if so spot the error I made.
>
> Diff is attached, please note that 'patch' did not make a very
> readable diff this time.

One thing I noticed is that you have (accidentally) moved the 
initialization of mReadOnly after readConfig().

-  mReadOnly = readonly;
   mConfig = new KConfig( "emailidentities" );
-  readConfig( mConfig );
+  d->readConfig( mConfig );
+  d->readOnly = readonly;


Another thing that might cause problems is that the virtual method 
IdentityManager::createDefaultIdentity() is called (indirectly via 
IdentityManager::Private::createDefaultIdentity()) in the c'tor 
IdentityManager::IdentityManager(). This will not work as expected 
because by the time IdentityManager::IdentityManager() is executed a 
subclass derived from IdentityManager will not yet have been fully 
initialized. But since d->createDefaultIdentity() is called after 
d->readConfig( mConfig ) this does not explain the crash.


I suggest to move mIdentities and mShadowIdentities also to 
IdentityManager::Private. And why is mConfig not in Private? If you 
want to use pimpl then please don't do it half-heartedly.


The following code snippet is really ugly (which isn't your fault):
    q->mIdentities << Identity();
    q->mIdentities.last().readConfig( configGroup );
    if ( !haveDefault && q->mIdentities.last().uoid() == 
defaultIdentity ) {
      haveDefault = true;
      q->mIdentities.last().setIsDefault( true );
    }

Instead of using QList::last() all the time I'd use a temporary 
variable:
    Identity identity;
    identity.readConfig( configGroup );
    if ( !haveDefault && identity.uoid() == defaultIdentity ) {
      haveDefault = true;
      identity.setIsDefault( true );
    }
    q->mIdentities << identity;


Regards,
Ingo

["signature.asc" (application/pgp-signature)]

_______________________________________________
KDE PIM mailing list kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/

[prev in list] [next in list] [prev in thread] [next in thread]