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

[Ingo Klöcker]

On Tuesday 20 May 2008, Jaroslaw Staniek wrote:
> 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;
> }

Or just

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

I guess it doesn't really make much of a difference. deleteLater() will 
most likely have no effect because event processing will already have 
been stopped when s_slavePool is destroyed.


Regards,
Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20080520/57e51eae/attachment.sig>
-------------- next part --------------
_______________________________________________
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/