QCString construction

[Ingo Klöcker]

On Monday 12 February 2007 16:21, David Faure wrote:
> On Sunday 11 February 2007, Ingo Klöcker wrote:
> > On Saturday 10 February 2007 02:16, David Faure wrote:
> > > Can a kmail developer review the change to KMMessage::asString()
> > > and asSendableString(), too? It avoids a asString() (Assemble)
> > > and a fromString (Parse), but I hope it's doing the right thing.
> >
> > I am very uneasy about those changes because completely different
> > things happen when you copy a message and when you create a message
> > from a string.
>
> Yeah, same here, that's what I asked.
>
> > If we had unit tests... But as it stands I'm against those
> > changes.
>
> OK I made it at least
>   KMMessage msg;
>   msg.fromDwString(asDwString());
> which saves a DwString -> QCString -> DwString roundtrip.

Good.

> The attached patch has many similar improvements.
> Things like calling asDwString instead of asString in
> KMComposeWin::autoSaveMessage are a simple, safe, and huge
> improvement in memory usage (found that one because I got an "out of
> memory" abort from that code, after attaching a large file). Then I
> grepped for other calls to asString and converted the relevant ones
> too.
>
> The only part I'm a bit unsure about is strings with embedded nuls.
> E.g. in bodyDecoded() they would already trigger a kdWarning, which I
> commented out because calling the slow QCString::length() just for
> that isn't worth it, but this means that later on when using size()-1
> instead of length() we actually get the full data instead of stopping
> at the first nil... Sounds like a bugfix rather than a bug, but it's
> certainly a behavior change. Not sure how to trigger it to test the
> side effects though.

There is a bug report about KMail truncating messages at the first \0. 
So, yeah, I agree it's (part of) a fix for this bug.

The following doesn't look right:
+QCString KMail::Util::CString( const DwString& str )
+{
+  const int strLen = str.size();
+  QCString cstr( strLen + 1 );
+  memcpy( cstr.data(), str.data(), strLen + 1 );
+  cstr[ strLen ] = 0;

I think it should be
+  memcpy( cstr.data(), str.data(), strLen );

Everything else looks good.

Ideally, we'd probably change all usage of QCString to DwString. But I'm 
not sure whether it's worth putting much effort into this since for KDE 
4.x we'll probably use QByteArray (with KMime instead of mimelib) 
everywhere.

Regards,
Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-optimize/attachments/20070212/3afad1bf/attachment.pgp