Re: QCString construction

--===============0192254833==
Content-type: multipart/signed; boundary=nextPart1450317.hW9KTD4GZc;
 protocol="application/pgp-signature"; micalg=pgp-sha1
Content-transfer-encoding: 7bit

--nextPart1450317.hW9KTD4GZc
Content-Type: text/plain;
  charset="iso-8859-15"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

On Monday 12 February 2007 16:21, David Faure wrote:
> On Sunday 11 February 2007, Ingo Kl=F6cker 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.=20
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 =3D str.size();
+  QCString cstr( strLen + 1 );
+  memcpy( cstr.data(), str.data(), strLen + 1 );
+  cstr[ strLen ] =3D 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=20
not sure whether it's worth putting much effort into this since for KDE=20
4.x we'll probably use QByteArray (with KMime instead of mimelib)=20
everywhere.

Regards,
Ingo

--nextPart1450317.hW9KTD4GZc
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)

iD8DBQBF0M6BGnR+RTDgudgRAkqrAKDWM8mXfkeN5NLTzyEqRvo2qq+r/wCfR37X
NtePLtLUGwLVeA0kI5ZPJKw=
=lity
-----END PGP SIGNATURE-----

--nextPart1450317.hW9KTD4GZc--


--===============0192254833==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Kde-optimize mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-optimize

--===============0192254833==--