[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: KConfigGroup read/write bug for QByteArray
From:       David Jarvie <lists () astrojar ! org ! uk>
Date:       2007-09-17 1:39:33
Message-ID: 200709170239.33527.lists () astrojar ! org ! uk
[Download RAW message or body]

On Sunday 16 September 2007 14:41:28 Matt Rogers wrote:
> On Sunday 16 September 2007 07:41, David Jarvie wrote:
> > Using KConfigGroup::writeEntry() to write a QByteArray and then
> > KConfigGroup::readEntry() to read it back in again can result in a value
> > which is different from the original value, because the value written to
> > the config file is converted to UTF8 when it is read back, so that all
> > byte values >= 0x7F are changed to multi-byte values.
> >
> > If this is the desired behaviour (which I doubt), it's difficult to
> > convert back from UTF8 since calling QString::fromUtf8 truncates the
> > value as soon as it encounters a null byte. In data arrays, null bytes
> > are common.
>
> Can you provide a patch? Then we can discuss your proposal based on the
> technical merits of it rather than just sitting in our chairs wondering to
> ourselves how it would actually work and whether or not we'd like it.

It turns out that the bug actually lies in KConfigGroup::writeEntry(), and 
that all that is required is for a QByteArray not to be converted to UTF8 
before being written. I attach a patch to fix it, and to add a test case to 
the unit test.

-- 
David Jarvie.
KAlarm author and maintainer.
http://www.astrojar.org.uk/kalarm

["bytearray.diff" (text/x-diff)]

Index: tests/kconfigtest.cpp
===================================================================
--- tests/kconfigtest.cpp	(revision 708428)
+++ tests/kconfigtest.cpp	(working copy)
@@ -39,6 +39,7 @@
 #define STRINGENTRY5 " "
 #define STRINGENTRY6 ""
 #define UTF8BITENTRY "Hello äöü"
+#define BYTEARRAYENTRY QByteArray( "\x00\xff\x7f\x3c abc\x00\x00", 10 )
 #define ESCAPEKEY " []\0017[]==]"
 #define ESCAPEENTRY "[]\170[]]=3=]\\] "
 #define DOUBLEENTRY 123456.78912345
@@ -72,6 +73,7 @@
   QCOMPARE( data.size(), 12 ); // the source file is in utf8
   QCOMPARE( QString::fromUtf8(data).length(), 9 );
   cg.writeEntry( "Test", QVariant( data ) ); // passing "data" converts it to char* \
and KConfigBase calls fromLatin1! +  cg.writeEntry( "bytearrayEntry", BYTEARRAYENTRY \
);  cg.writeEntry( ESCAPEKEY, ESCAPEENTRY );
   cg.writeEntry( "Test2", "");
   cg.writeEntry( "stringEntry1", STRINGENTRY1 );
@@ -207,6 +209,7 @@
 
   sc3 = KConfigGroup(&sc2, "Hello");
   QCOMPARE( sc3.readEntry( "Test", QByteArray() ), QByteArray( UTF8BITENTRY ) );
+  QCOMPARE( sc3.readEntry( "bytearrayEntry", QByteArray() ), BYTEARRAYENTRY );
   QCOMPARE( sc3.readEntry( ESCAPEKEY ), QString( ESCAPEENTRY ) );
   QCOMPARE( sc3.readEntry( "Test", QString() ), QString::fromUtf8( UTF8BITENTRY ) );
   QCOMPARE( sc3.readEntry("Test2", QString("Fietsbel")).isEmpty(), true );
Index: config/kconfiggroup.cpp
===================================================================
--- config/kconfiggroup.cpp	(revision 708428)
+++ config/kconfiggroup.cpp	(working copy)
@@ -681,6 +681,31 @@
 void KConfigGroup::writeEntry( const char *pKey, const QString& value,
                                KConfigBase::WriteConfigFlags pFlags )
 {
+    writeEntry( pKey, value.toUtf8(), pFlags );
+}
+
+void KConfigGroup::writeEntry( const QString& pKey, const QStringList &value,
+                 char sep,
+                 WriteConfigFlags pFlags )
+{
+    writeEntry( pKey.toUtf8().constData(), value, sep, pFlags );
+}
+
+void KConfigGroup::writeEntry( const char* pKey,
+        const QVariantList& value, WriteConfigFlags pFlags )
+{
+    writeEntry( pKey, QVariant(value), pFlags );
+}
+
+void KConfigGroup::writeEntry( const char *pKey, const char *value,
+                     WriteConfigFlags pFlags )
+{
+    writeEntry(pKey, QString::fromLatin1(value), pFlags);
+}
+
+void KConfigGroup::writeEntry( const char *pKey, const QByteArray& value,
+                     WriteConfigFlags pFlags )
+{
     // the KConfig object is dirty now
     // set this before any IO takes place so that if any derivative
     // classes do caching, they won't try and flush the cache out
@@ -698,7 +723,7 @@
     entryKey.bLocal = pFlags & KConfigBase::NLS;
 
     KEntry aEntryData;
-    aEntryData.mValue = value.toUtf8();  // set new value
+    aEntryData.mValue = value;  // set new value
     aEntryData.bGlobal = pFlags & KConfigBase::Global;
     aEntryData.bNLS = pFlags & KConfigBase::NLS;
 
@@ -710,31 +735,6 @@
     putData(entryKey, aEntryData, true);
 }
 
-void KConfigGroup::writeEntry( const QString& pKey, const QStringList &value,
-                 char sep,
-                 WriteConfigFlags pFlags )
-{
-    writeEntry( pKey.toUtf8().constData(), value, sep, pFlags );
-}
-
-void KConfigGroup::writeEntry( const char* pKey,
-        const QVariantList& value, WriteConfigFlags pFlags )
-{
-    writeEntry( pKey, QVariant(value), pFlags );
-}
-
-void KConfigGroup::writeEntry( const char *pKey, const char *value,
-                     WriteConfigFlags pFlags )
-{
-    writeEntry(pKey, QString::fromLatin1(value), pFlags);
-}
-
-void KConfigGroup::writeEntry( const char *pKey, const QByteArray& value,
-                     WriteConfigFlags pFlags )
-{
-    writeEntry(pKey, QString::fromLatin1(value, value.size()), pFlags);
-}
-
 void KConfigGroup::writePathEntry( const QString& pKey, const QString & path,
         KConfigBase::WriteConfigFlags pFlags )
 {
@@ -888,8 +888,7 @@
       writeEntry( pKey, prop.toStringList(), ',', pFlags );
       return;
     case QVariant::ByteArray: {
-      const QByteArray ba = prop.toByteArray();
-      writeEntry( pKey, QString::fromUtf8(ba.constData(), ba.length()), pFlags );
+      writeEntry( pKey, prop.toByteArray(), pFlags );
       return;
     }
     case QVariant::Point: {



[prev in list] [next in list] [prev in thread] [next in thread]