QCString construction

David Faure dfaure at klaralvdalens-datakonsult.se
Mon Feb 12 16:21:55 CET 2007


On Sunday 11 February 2007, Ingo Klöcker wrote:
> On Saturday 10 February 2007 02:16, David Faure wrote:
> > This leads me to 3 patches.
> >
> > One for kmail, I wrote a utility function for creating a QCString
> > from char*+size, and used that when creating a QCString from a
> > DwString where it matters (i.e. where I found pretty large strings to
> > be used when attaching large files).
> 
> The related changes look good. I'm just wondering why you didn't add
>   QCString CString( const DwString & str )
> instead of (or additionally to)
>   QCString CString( const char* str, size_t strLen )
> ?

You're right, it makes the code simpler. This came from my unit test which was DwString-independent,
and from the idea that maybe we have more cases were we know the string size, but in fact we don't.

> > 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.

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.

-- 
David Faure, faure at kde.org, dfaure at klaralvdalens-datakonsult.se
KDE/KOffice developer, Qt consultancy projects
Klarälvdalens Datakonsult AB, Platform-independent software solutions
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kmail.diff
Type: text/x-diff
Size: 17502 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-optimize/attachments/20070212/14ed432b/attachment-0001.bin 


More information about the Kde-optimize mailing list