[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-optimize
Subject: Re: QCString construction
From: David Faure <dfaure () klaralvdalens-datakonsult ! se>
Date: 2007-02-12 15:21:55
Message-ID: 200702121622.15291.dfaure () klaralvdalens-datakonsult ! se
[Download RAW message or body]
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@kde.org, dfaure@klaralvdalens-datakonsult.se
KDE/KOffice developer, Qt consultancy projects
Klarälvdalens Datakonsult AB, Platform-independent software solutions
["kmail.diff" (text/x-diff)]
Index: util.cpp
===================================================================
--- util.cpp (revision 632791)
+++ util.cpp (working copy)
@@ -39,6 +39,7 @@
#include <stdlib.h>
#include <qcstring.h>
+#include <mimelib/string.h>
size_t KMail::Util::crlf2lf( char* str, const size_t strLen )
{
@@ -87,3 +88,12 @@ QCString KMail::Util::lf2crlf( const QCS
result.truncate( d - result.begin() ); // adds trailing NUL
return result;
}
+
+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;
+ return cstr;
+}
Index: kmmsgpart.cpp
===================================================================
--- kmmsgpart.cpp (revision 632791)
+++ kmmsgpart.cpp (working copy)
@@ -10,6 +10,7 @@
#include "kmkernel.h"
#include "kmmessage.h"
#include "globalsettings.h"
+#include "util.h"
#include <kasciistringtools.h>
#include <kmime_charfreq.h>
@@ -102,7 +103,18 @@ int KMMessagePart::decodedSize(void) con
//-----------------------------------------------------------------------------
void KMMessagePart::setBody(const QCString &aStr)
{
- mBody.duplicate( aStr.data(), aStr.length() );
+ KMail::Util::setFromQCString( mBody, aStr );
+
+ int enc = cte();
+ if (enc == DwMime::kCte7bit || enc == DwMime::kCte8bit || enc == DwMime::kCteBinary)
+ mBodyDecodedSize = mBody.size();
+ else
+ mBodyDecodedSize = -1; // Can't know the decoded size
+}
+
+void KMMessagePart::setBody(const DwString &aStr)
+{
+ mBody.duplicate( aStr.c_str(), aStr.length() );
int enc = cte();
if (enc == DwMime::kCte7bit || enc == DwMime::kCte8bit || enc == DwMime::kCteBinary)
@@ -161,8 +173,7 @@ void KMMessagePart::setCharset( const QC
//-----------------------------------------------------------------------------
void KMMessagePart::setBodyEncoded(const QCString& aStr)
{
- mBodyDecodedSize = aStr.length();
-
+ mBodyDecodedSize = aStr.size() - 1; // same as aStr.length() but faster - assuming no embedded nuls
switch (cte())
{
case DwMime::kCteQuotedPrintable:
@@ -224,7 +235,7 @@ void KMMessagePart::setBodyAndGuessCte(c
bool allow8Bit,
bool willBeSigned )
{
- mBodyDecodedSize = aBuf.length();
+ mBodyDecodedSize = aBuf.size() - 1; // same as aStr.length() but faster - assuming no embedded nuls
CharFreq cf( aBuf.data(), mBodyDecodedSize ); // save to pass null strings
@@ -348,13 +359,12 @@ QCString KMMessagePart::bodyDecoded(void
if ( decodeBinary ) {
len = mBody.size();
- result.resize( len+1 /* trailing NUL */ );
- memcpy(result.data(), mBody.data(), len);
- result[len] = 0;
+ KMail::Util::setFromByteArray( result, mBody );
}
- kdWarning( result.length() != (unsigned int)len, 5006 )
- << "KMMessagePart::bodyDecoded(): body is binary but used as text!" << endl;
+ // Calls length -> slow
+ //kdWarning( result.length() != (unsigned int)len, 5006 )
+ // << "KMMessagePart::bodyDecoded(): body is binary but used as text!" << endl;
result = result.replace( "\r\n", "\n" ); // CRLF -> LF conversion
@@ -544,10 +554,12 @@ QString KMMessagePart::fileName(void) co
return QString::null;
}
-
-
QCString KMMessagePart::body() const
{
return QCString( mBody.data(), mBody.size() + 1 ); // space for trailing NUL
}
+DwString KMMessagePart::dwBody() const
+{
+ return DwString( mBody.data(), mBody.size() );
+}
Index: kmfoldermaildir.cpp
===================================================================
--- kmfoldermaildir.cpp (revision 632791)
+++ kmfoldermaildir.cpp (working copy)
@@ -406,7 +406,7 @@ if( fileD0.open( IO_WriteOnly ) ) {
if ( !uidHeader.isEmpty() && stripUid )
aMsg->removeHeaderField( "X-UID" );
- msgText = aMsg->asString();
+ msgText = aMsg->asString(); // TODO use asDwString instead
len = msgText.length();
// Re-add the uid so that the take can make use of it, in case the
Index: kmreaderwin.cpp
===================================================================
--- kmreaderwin.cpp (revision 632791)
+++ kmreaderwin.cpp (working copy)
@@ -1614,13 +1614,13 @@ kdDebug(5006) << "KMReaderWin - compos
unencryptedMessage->setParent( 0 );
// because this did not work:
/*
- DwMessage dwMsg( DwString( aMsg->asString() ) );
+ DwMessage dwMsg( aMsg->asDwString() );
dwMsg.Body() = DwBody( DwString( resultString.data() ) );
dwMsg.Body().Parse();
KMMessage* unencryptedMessage = new KMMessage( &dwMsg );
*/
-kdDebug(5006) << "KMReaderWin - resulting message:" << unencryptedMessage->asString() << endl;
-kdDebug(5006) << "KMReaderWin - attach unencrypted message to aMsg" << endl;
+ //kdDebug(5006) << "KMReaderWin - resulting message:" << unencryptedMessage->asString() << endl;
+ kdDebug(5006) << "KMReaderWin - attach unencrypted message to aMsg" << endl;
aMsg->setUnencryptedMsg( unencryptedMessage );
emitReplaceMsgByUnencryptedVersion = true;
}
Index: partNode.cpp
===================================================================
--- partNode.cpp (revision 632791)
+++ partNode.cpp (working copy)
@@ -37,6 +37,7 @@
#include <mimelib/utility.h>
#include <qregexp.h>
#include <kasciistricmp.h>
+#include "util.h"
/*
===========================================================================
@@ -179,7 +180,7 @@ const QCString & partNode::encodedBody()
return mEncodedBody;
if ( mDwPart )
- mEncodedBody = mDwPart->Body().AsString().c_str();
+ mEncodedBody = KMail::Util::CString( mDwPart->Body().AsString() );
else
mEncodedBody = 0;
mEncodedOk = true;
Index: kmmessage.cpp
===================================================================
--- kmmessage.cpp (revision 632791)
+++ kmmessage.cpp (working copy)
@@ -64,6 +64,7 @@ using namespace KMime::Types;
#include <klocale.h>
#include <stdlib.h>
#include <unistd.h>
+#include "util.h"
#if ALLOW_GUI
#include <kmessagebox.h>
@@ -212,7 +213,7 @@ QCString KMMessage::id() const
{
DwHeaders& header = mMsg->Headers();
if (header.HasMessageId())
- return header.MessageId().AsString().c_str();
+ return KMail::Util::CString( header.MessageId().AsString() );
else
return "";
}
@@ -306,14 +307,14 @@ const DwMessage *KMMessage::asDwMessage(
//-----------------------------------------------------------------------------
QCString KMMessage::asString() const {
- return asDwString().c_str();
+ return KMail::Util::CString( asDwString() );
}
QCString KMMessage::asSendableString() const
{
KMMessage msg;
- msg.fromString(asString());
+ msg.fromDwString(asDwString()); // slow!
msg.removePrivateHeaderFields();
msg.removeHeaderField("Bcc");
return msg.asString();
@@ -322,7 +323,7 @@ QCString KMMessage::asSendableString() c
QCString KMMessage::headerAsSendableString() const
{
KMMessage msg;
- msg.fromString(asString());
+ msg.fromDwString(asDwString());
msg.removePrivateHeaderFields();
msg.removeHeaderField("Bcc");
return msg.headerAsString().latin1();
@@ -392,7 +393,7 @@ void KMMessage::fromByteArray( const QBy
}
void KMMessage::fromString( const QCString & str, bool aSetStatus ) {
- return fromDwString( DwString( str.data() ), aSetStatus );
+ return fromDwString( DwString( str.data(), str.size() ), aSetStatus );
}
void KMMessage::fromDwString(const DwString& str, bool aSetStatus)
@@ -1250,7 +1251,7 @@ KMMessage* KMMessage::createForward( con
KMMessagePart secondPart;
secondPart.setType( type() );
secondPart.setSubtype( subtype() );
- secondPart.setBody( mMsg->Body().AsString().c_str() );
+ secondPart.setBody( mMsg->Body().AsString() );
// use the headers of the original mail
applyHeadersToMessagePart( mMsg->Headers(), &secondPart );
msg->addBodyPart(&secondPart);
@@ -1761,9 +1762,9 @@ QCString KMMessage::dateShortStr() const
unixTime = header.Date().AsUnixTime();
QCString result = ctime(&unixTime);
-
- if (result[result.length()-1]=='\n')
- result.truncate(result.length()-1);
+ int len = result.length();
+ if (result[len-1]=='\n')
+ result.truncate(len-1);
return result;
}
@@ -2469,10 +2470,11 @@ void KMMessage::setNeedsAssembly()
//-----------------------------------------------------------------------------
QCString KMMessage::body() const
{
- DwString body = mMsg->Body().AsString();
- QCString str = body.c_str();
- kdWarning( str.length() != body.length(), 5006 )
- << "KMMessage::body(): body is binary but used as text!" << endl;
+ const DwString& body = mMsg->Body().AsString();
+ QCString str = KMail::Util::CString( body );
+ // Calls length() -> slow
+ //kdWarning( str.length() != body.length(), 5006 )
+ // << "KMMessage::body(): body is binary but used as text!" << endl;
return str;
}
@@ -2481,7 +2483,7 @@ QCString KMMessage::body() const
QByteArray KMMessage::bodyDecodedBinary() const
{
DwString dwstr;
- DwString dwsrc = mMsg->Body().AsString();
+ const DwString& dwsrc = mMsg->Body().AsString();
switch (cte())
{
@@ -2522,13 +2524,13 @@ QCString KMMessage::bodyDecoded() const
break;
}
- unsigned int len = dwstr.size();
- QCString result(len+1);
- memcpy(result.data(),dwstr.data(),len);
- result[len] = 0;
- kdWarning(result.length() != len, 5006)
- << "KMMessage::bodyDecoded(): body is binary but used as text!" << endl;
- return result;
+ return KMail::Util::CString( dwstr );
+
+ // Calling QCString::length() is slow
+ //QCString result = KMail::Util::CString( dwstr );
+ //kdWarning(result.length() != len, 5006)
+ // << "KMMessage::bodyDecoded(): body is binary but used as text!" << endl;
+ //return result;
}
@@ -2609,7 +2611,7 @@ void KMMessage::setBodyAndGuessCte( cons
bool allow8Bit,
bool willBeSigned )
{
- CharFreq cf( aBuf.data(), aBuf.length() ); // it's safe to pass null strings
+ CharFreq cf( aBuf.data(), aBuf.size()-1 ); // it's safe to pass null strings
allowedCte = determineAllowedCtes( cf, allow8Bit, willBeSigned );
@@ -2964,9 +2966,9 @@ void KMMessage::bodyPart(DwBodyPart* aDw
// Body
if (withBody)
- aPart->setBody( aDwBodyPart->Body().AsString().c_str() );
+ aPart->setBody( aDwBodyPart->Body().AsString() );
else
- aPart->setBody( "" );
+ aPart->setBody( QCString("") );
// Content-id
if ( headers.HasContentId() ) {
@@ -2987,7 +2989,7 @@ void KMMessage::bodyPart(DwBodyPart* aDw
//aPart->setName(" ");
aPart->setContentDescription("");
aPart->setContentDisposition("");
- aPart->setBody("");
+ aPart->setBody(QCString(""));
aPart->setContentId("");
}
}
@@ -3135,8 +3137,9 @@ DwBodyPart* KMMessage::createDWBodyPart(
if (!contDisp.isEmpty())
headers.ContentDisposition().FromString(contDisp);
- if (!aPart->body().isNull())
- part->Body().FromString(aPart->body());
+ const DwString bodyStr = aPart->dwBody();
+ if (!bodyStr.empty())
+ part->Body().FromString(bodyStr);
else
part->Body().FromString("");
@@ -3194,7 +3197,7 @@ QString KMMessage::generateMessageId( co
//-----------------------------------------------------------------------------
QCString KMMessage::html2source( const QCString & src )
{
- QCString result( 1 + 6*src.length() ); // maximal possible length
+ QCString result( 1 + 6*(src.size()-1) ); // maximal possible length
QCString::ConstIterator s = src.begin();
QCString::Iterator d = result.begin();
Index: kmcommands.cpp
===================================================================
--- kmcommands.cpp (revision 632791)
+++ kmcommands.cpp (working copy)
@@ -692,7 +692,7 @@ KMCommand::Result KMUseTemplateCommand::
// Take a copy of the original message, which remains unchanged.
KMMessage *newMsg = new KMMessage;
newMsg->setComplete( msg->isComplete() );
- newMsg->fromString( msg->asString() );
+ newMsg->fromDwString( msg->asDwString() );
KMail::Composer *win = KMail::makeComposer();
newMsg->setTransferInProgress( false ); // From here on on, the composer owns the message.
@@ -1923,7 +1923,7 @@ KMCommand::Result KMCopyCommand::execute
// make sure the attachment state is only calculated when it's complete
if (!newMsg->isComplete())
newMsg->setReadyToShow(false);
- newMsg->fromString(msg->asString());
+ newMsg->fromDwString(msg->asDwString());
newMsg->setStatus(msg->status());
if (srcFolder && !newMsg->isComplete())
Index: messagecomposer.cpp
===================================================================
--- messagecomposer.cpp (revision 632791)
+++ messagecomposer.cpp (working copy)
@@ -1548,7 +1548,7 @@ void MessageComposer::composeMessage( KM
if ( it->encrypt == doEncryptBody && it->sign == doSignBody ) {
innerDwPart = theMessage.createDWBodyPart( it->part );
innerDwPart->Assemble();
- body += "\n--" + mMultipartMixedBoundary + "\n" + innerDwPart->AsString().c_str();
+ body += "\n--" + mMultipartMixedBoundary + "\n" + innerDwPart->AsString().c_str(); // SLOW!
delete innerDwPart;
innerDwPart = 0;
}
@@ -1577,13 +1577,12 @@ void MessageComposer::composeMessage( KM
DwMediaType& ct = headers.ContentType();
ct.SetBoundary(mSaveBoundary);
dwPart->Assemble();
- mEncodedBody = dwPart->AsString().c_str();
}
else {
dwPart = theMessage.createDWBodyPart( &mOldBodyPart );
dwPart->Assemble();
- mEncodedBody = dwPart->AsString().c_str();
}
+ mEncodedBody = KMail::Util::CString( dwPart->AsString() );
delete dwPart;
dwPart = 0;
@@ -1784,7 +1783,7 @@ void MessageComposer::addBodyAndAttachme
DwBodyPart* innerDwPart = msg->createDWBodyPart( it->part );
innerDwPart->Assemble();
- QCString encodedAttachment = innerDwPart->AsString().c_str();
+ QCString encodedAttachment = KMail::Util::CString( innerDwPart->AsString() );
delete innerDwPart;
innerDwPart = 0;
@@ -1815,7 +1814,7 @@ void MessageComposer::addBodyAndAttachme
rEncryptMessagePart = newAttachPart;
DwBodyPart* dwPart = msg->createDWBodyPart( &newAttachPart );
dwPart->Assemble();
- encodedAttachment = dwPart->AsString().c_str();
+ encodedAttachment = KMail::Util::CString( dwPart->AsString() );
delete dwPart;
dwPart = 0;
}
Index: kmcomposewin.cpp
===================================================================
--- kmcomposewin.cpp (revision 632802)
+++ kmcomposewin.cpp (working copy)
@@ -707,8 +707,8 @@ void KMComposeWin::autoSaveMessage()
if ( status == 0 ) { // no error
kdDebug(5006) << "autosaving message in " << filename << endl;
int fd = autoSaveFile.handle();
- QCString msgStr = msg->asString();
- if ( ::write( fd, msgStr, msgStr.length() ) == -1 )
+ const DwString& msgStr = msg->asDwString();
+ if ( ::write( fd, msgStr.data(), msgStr.length() ) == -1 )
status = errno;
}
if ( status == 0 ) {
Index: util.h
===================================================================
--- util.h (revision 632791)
+++ util.h (working copy)
@@ -41,6 +41,7 @@
#include <stdlib.h>
#include <qobject.h>
#include <qcstring.h>
+class DwString;
namespace KMail
{
@@ -68,8 +69,39 @@ namespace Util {
QCString lf2crlf( const QCString & src );
/**
+ * Construct a QCString from a DwString
+ */
+ QCString CString( const DwString& str );
+
+ /**
+ * Fills a QByteArray from a QCString - removing the trailing null.
+ */
+ void setFromQCString( QByteArray& arr, const QCString& cstr );
+
+ inline void setFromQCString( QByteArray& arr, const QCString& cstr )
+ {
+ if ( cstr.size() )
+ arr.duplicate( cstr.data(), cstr.size()-1 );
+ else
+ arr.resize(0);
+ }
+
+ /**
+ * Fills a QCString from a QByteArray - adding the trailing null.
+ */
+ void setFromByteArray( QCString& cstr, const QByteArray& arr );
+
+ inline void setFromByteArray( QCString& result, const QByteArray& arr )
+ {
+ const int len = arr.size();
+ result.resize( len + 1 /* trailing NUL */ );
+ memcpy(result.data(), arr.data(), len);
+ result[len] = 0;
+ }
+
+ /**
* A LaterDeleter is intended to be used with the RAII ( Resource
- * Acquisiation is Initialization ) paradigm. When an instance of it
+ * Acquisition is Initialization ) paradigm. When an instance of it
* goes out of scope it deletes the associated object It can be
* disabled, in case the deletion needs to be avoided for some
* reason, since going out-of-scope cannot be avoided.
Index: kmmsgpart.h
===================================================================
--- kmmsgpart.h (revision 632791)
+++ kmmsgpart.h (working copy)
@@ -27,6 +27,7 @@
template <typename T>
class QValueList;
class QTextCodec;
+class DwString;
class KMMessagePart
{
@@ -45,6 +46,8 @@ public:
/** Get or set the message body */
QCString body(void) const;
void setBody(const QCString &aStr);
+ DwString dwBody() const;
+ void setBody(const DwString &aStr);
/** Sets this body part's content to @p str. @p str is subject to
automatic charset and CTE detection.
_______________________________________________
Kde-optimize mailing list
Kde-optimize@kde.org
https://mail.kde.org/mailman/listinfo/kde-optimize
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic