[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: Re: KDE/kdelibs/kdecore/config
From: Jakub Stachowski <stachowski () hypair ! net>
Date: 2008-04-27 13:15:58
Message-ID: 200804271515.58619.stachowski () hypair ! net
[Download RAW message or body]
Dnia niedziela, 27 kwietnia 2008, Oswald Buddenhagen napisaĆ:
> reviewing it again ...
>
> On Mon, Apr 21, 2008 at 02:01:58PM +0000, Jakub Stachowski wrote:
> > SVN commit 799413 by qbast:
> >
> > Ini parsing optimization - avoid using QByteArray and excessive memory
> > copying.
> >
> > --- trunk/KDE/kdelibs/kdecore/config/kconfigini.cpp #799412:799413
> > @@ -165,17 +174,17 @@
> > if (groupOptionImmutable)
> > entryOptions |= KEntryMap::EntryImmutable;
> >
> > BufferFragment locale;
> > BufferFragment rawKey;
> > int start;
> > - while ((start = aKey.indexOf('[')) >= 0) {
> > + while ((start = aKey.lastIndexOf('[')) >= 0) {
> > int end = aKey.indexOf(']', start);
> > @@ -188,8 +197,9 @@
> > break;
> > case 'd':
> > entryOptions |= KEntryMap::EntryDeleted;
> > - aKey =
> > printableToString(aKey.left(start), file, lineNo); -
> > entryMap.setEntry(currentGroup, aKey, QByteArray(),
> > entryOptions); + aKey = aKey.left(start);
> > + printableToString(&aKey, file, lineNo);
> > + entryMap.setEntry(currentGroup,
> > aKey.toByteArray(), QByteArray(), entryOptions); goto next_line;
> > default:
> > break;
> > @@ -203,17 +213,16 @@
> > goto next_line;
> > }
> >
> > locale = aKey.mid(start + 1,end - start - 1);
> > rawKey = aKey.left(end + 1);
> > }
> > - aKey.remove(start, end-start+1);
> > + aKey.truncate(start);
> > }
>
> i *think* you broke the case where the immutability marker comes before
> the locale marker:
>
> Key[$i][de_DE]=foo
>
> care to craft a test?
My changes completely botched rawKey, so line like entry[$i][fr]=test after
sync would become entry[$i][fr][$i]=test :-(
Attached fix (with unittest) fixes this behaviour by recreating raw key from
parsed key (without unescaping) +'['+locale+']'. As a bonus it is about ~1%
faster (due to omitting to printableToString for raw keys).
Do you still see anything suspicious?
["localefix.patch" (text/x-diff)]
Index: tests/kconfigtest.cpp
===================================================================
--- tests/kconfigtest.cpp (wersja 799070)
+++ tests/kconfigtest.cpp (kopia robocza)
@@ -773,6 +773,45 @@
QVERIFY(cg2.isImmutable());
}
+void KConfigTest::testOptionOrder()
+{
+ {
+ QFile file(KStandardDirs::locateLocal("config", "doubleattrtest"));
+ file.open(QIODevice::WriteOnly|QIODevice::Text);
+ QTextStream out(&file);
+ out.setCodec("UTF-8");
+ out << "[group3]" << endl
+ << "entry2=unlocalized" << endl
+ << "entry2[$i][de_DE]=t2" << endl;
+ }
+ KConfig config("doubleattrtest", KConfig::SimpleConfig);
+ config.setLocale("de_DE");
+ KConfigGroup cg3 = config.group("group3");
+ QVERIFY(!cg3.isImmutable());
+ QCOMPARE(cg3.readEntry("entry2",""), QString("t2"));
+ QVERIFY(cg3.isEntryImmutable("entry2"));
+ config.setLocale("C");
+ QCOMPARE(cg3.readEntry("entry2",""), QString("unlocalized"));
+ QVERIFY(!cg3.isEntryImmutable("entry2"));
+ cg3.writeEntry("entry2","modified");
+ config.sync();
+
+ {
+ QList<QByteArray> lines;
+ // this is what the file should look like
+ lines << "[group3]\n"
+ << "entry2=modified\n"
+ << "entry2[de_DE][$i]=t2\n";
+
+ QFile file(KStandardDirs::locateLocal("config", "doubleattrtest"));
+ file.open(QIODevice::ReadOnly|QIODevice::Text);
+ foreach (const QByteArray& line, lines) {
+ QCOMPARE(line, file.readLine());
+ }
+ }
+}
+
+
void KConfigTest::testGroupEscape()
{
KConfig config("groupescapetest", KConfig::SimpleConfig);
Index: tests/kconfigtest.h
===================================================================
--- tests/kconfigtest.h (wersja 799070)
+++ tests/kconfigtest.h (kopia robocza)
@@ -63,6 +63,7 @@
void testWriteOnSync();
void testCreateDir();
void testSharedConfig();
+ void testOptionOrder();
// unrelated
void testKAboutDataOrganizationDomain();
Index: config/kconfigini.cpp
===================================================================
--- config/kconfigini.cpp (wersja 801679)
+++ config/kconfigini.cpp (kopia robocza)
@@ -175,7 +175,6 @@
entryOptions |= KEntryMap::EntryImmutable;
BufferFragment locale;
- BufferFragment rawKey;
int start;
while ((start = aKey.lastIndexOf('[')) >= 0) {
int end = aKey.indexOf(']', start);
@@ -214,7 +213,6 @@
}
locale = aKey.mid(start + 1,end - start - 1);
- rawKey = aKey.left(end + 1);
}
aKey.truncate(start);
}
@@ -227,16 +225,17 @@
if (locale != currentLocale) {
// backward compatibility. C == en_US
if (locale.at(0) != 'C' || currentLocale != "en_US") {
- if (merging){
+ if (merging)
entryOptions |= KEntryMap::EntryRawKey;
- aKey = rawKey; // store as unprocessed key
- locale = BufferFragment();
- } else
+ else
goto next_line; // skip this entry if we're not merging
}
}
}
+ if (!(entryOptions&KEntryMap::EntryRawKey))
+ printableToString(&aKey, file, lineNo);
+
if (options&ParseGlobal)
entryOptions |= KEntryMap::EntryGlobal;
if (bDefault)
@@ -244,7 +243,14 @@
if (!locale.isNull())
entryOptions |= KEntryMap::EntryLocalized;
printableToString(&line, file, lineNo);
- entryMap.setEntry(currentGroup, aKey.toByteArray(), line.toByteArray(), entryOptions);
+ if (entryOptions&KEntryMap::EntryRawKey) {
+ QByteArray rawKey;
+ rawKey.reserve(aKey.length()+locale.length()+2);
+ rawKey.append(aKey.toSharedByteArray()).append('[');
+ rawKey.append(locale.toSharedByteArray()).append(']');
+ entryMap.setEntry(currentGroup, rawKey, line.toByteArray(), entryOptions);
+ } else
+ entryMap.setEntry(currentGroup, aKey.toByteArray(), line.toByteArray(), entryOptions);
}
next_line:
continue;
Index: config/bufferfragment_p.h
===================================================================
--- config/bufferfragment_p.h (wersja 801679)
+++ config/bufferfragment_p.h (kopia robocza)
@@ -166,6 +166,12 @@
QByteArray toByteArray() const {
return QByteArray(d,len);
}
+
+ // this is faster than toByteArray, but returned QByteArray becomes invalid
+ // when buffer for this BufferFragment disappears
+ QByteArray toSharedByteArray() const {
+ return QByteArray::fromRawData(d, len);
+ }
private:
char* d;
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic