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