[Kde-pim] [patch] handling of static data in akonadi and mailtransport

[Jaroslaw Staniek]

Ingo Klöcker said the following, On 2008-05-20 20:02:
> On Tuesday 20 May 2008, Jarosław Staniek wrote:
>> Ingo Klöcker said the following, On 2008-05-20 00:14:
>>> On Monday 19 May 2008, Jarosław Staniek wrote:
>>>> Index: mailtransport/socket.cpp
>>>> ==================================================================
>>>> = --- mailtransport/socket.cpp    (wersja 809997)
>>>> +++ mailtransport/socket.cpp    (kopia robocza)
>>>> @@ -54,6 +54,8 @@
>>>>        void slotModeChanged( QSslSocket::SslMode  state );
>>>>        void slotSocketRead();
>>>>        void slotSslErrors( const QList<QSslError> &errors );
>>>> +    private:
>>>> +      QString msg;
>>> This should be
>>> +      QString m_msg;
>>>
>>>>    };
>>>>  }
>>>>
>>>> @@ -102,19 +104,19 @@
>>>>      return;
>>>>    }
>>>>
>>>> -  static QString msg;
>>>> -  msg += QLatin1String( socket->readAll() );
>>>> +  QString m_msg;
>>> The above line has to be deleted.
>> Sorry I must have been tired too. Attached updated patch.
>> Till, Bernhard: this is the newest patch to apply (after removing the
>> old one).
>>
>>>> +  m_msg += QLatin1String( socket->readAll() );
>>>>
>>>> -  if ( !msg.endsWith( QLatin1Char( '\n' ) ) ) {
>>>> +  if ( !m_msg.endsWith( QLatin1Char( '\n' ) ) ) {
>>>>      return;
>>>>    }
>>>>
>>>>  #ifdef comm_debug
>>>> -  kDebug() << socket->isEncrypted() << msg.trimmed();
>>>> +  kDebug() << socket->isEncrypted() << m_msg.trimmed();
>>>>  #endif
>>>>
>>>> -  emit q->data( msg );
>>>> -  msg.clear();
>>>> +  emit q->data( m_msg );
>>>> +  m_msg.clear();
>>>>  }
>>> Concerning mailtransport/smtpjob.cpp I'm wondering whether
>>> SlavePool can be destroyed before the last SmtpJob is destroyed. I
>>> guess it would be saner to make the last SmtpJob destroy the slave
>>> pool. The pool is anyway cleared each time the reference count
>>> drops to 0, so the slave pool could as well be destroyed at that
>>> point. And in the c'tor of SmtpJob the pool would be created if it
>>> does not exist. I don't think K_GLOBAL_STATIC is the correct tool
>>> for the job, but I'm too tired to really think about it right now.
>> This woud be extra code and allocation to save perhaps 10 bytes.
> 
> This isn't about saving a few bytes. If the global static s_slavePool is 
> destroyed before the last SmtpJob is destroyed then accessing 
> s_slavePool in SmtpJob::~SmtpJob() will cause a crash at program exit 
> (or, more precisely, it will cause a qFatal()).
> 
> I have no idea when those SmtpJobs are destroyed if they are still 
> running on program exit, so I don't know whether this crash can 
> actually happen. But better safe than sorry.
> 
> So please add
> 
>   if ( ! s_slavePool.isDestroyed() ) {
>     // okay, it is safe to access s_slavePool
>     ...
>   }
> 
> (without the comment) everywhere in SmtpJob where s_slavePool is 
> accessed.

Or just this at the beginning of each method? :

if ( s_slavePool.isDestroyed() ) {
	deleteLater();
	return;
}

-- 
regards / pozdrawiam, Jaroslaw Staniek
  Sponsored by OpenOffice Polska (http://www.openoffice.com.pl/en) to work on
  Kexi & KOffice (http://www.kexi.pl/en, http://www.koffice.org/kexi)
  KDE Libraries for MS Windows (http://windows.kde.org)
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/