Git commit 54938b1293ca9c7f3196ff854b3b60cf0a58175f by Andreas Hartmetz. Committed on 05/11/2016 at 15:57. Pushed by ahartmetz into branch 'master'. Changes to empty array handling, from usage experience. - Allow writing an empty array in the >1st iteration of another array. This is a straight bugfix - it has always been perfectly valid to do that. There was only no test for it. - Allow iterating through an empty array > 1 times when writing. The rationale is explained in a comment. M +16 -3 serialization/arguments.cpp M +53 -3 tests/serialization/tst_arguments.cpp M +0 -1 util/error.h http://commits.kde.org/dferry/54938b1293ca9c7f3196ff854b3b60cf0a58175f diff --git a/serialization/arguments.cpp b/serialization/arguments.cpp index 0e87009..2919219 100644 --- a/serialization/arguments.cpp +++ b/serialization/arguments.cpp @@ -2081,7 +2081,17 @@ void Arguments::Writer::advanceState(cstring signatu= reFragment, IoState newState } d->m_signature.length +=3D signatureFragment.length; } else { - VALID_IF(likely(!d->m_nilArrayNesting), Error::ExtraIterationInEmp= tyArray); + // Do not try to prevent several iterations through a nil array. T= wo reasons: + // - We may be writing a nil array in the >1st iteration of a non-= nil outer array. + // This would need to be distinguished from just iterating throu= gh a nil array + // several times. Which is well possible. We don't bother with t= hat because... + // - As a QtDBus unittest illustrates, somebody may choose to seri= alize a fixed length + // series of data elements as an array (instead of struct), so t= hat a trivial + // serialization of such data just to fill in type information i= n an outer empty array + // would end up iterating through the inner, implicitly empty ar= ray several times. + // All in all it is just not much of a benefit to be strict, so do= n't. + //VALID_IF(likely(!d->m_nilArrayNesting), Error::ExtraIterationInE= mptyArray); + // signature must match first iteration (of an array/dict) VALID_IF(d->m_signaturePosition + signatureFragment.length <=3D d-= >m_signature.length, Error::TypeMismatchInSubsequentArrayIteration); @@ -2236,13 +2246,16 @@ void Arguments::Writer::beginArrayOrDict(IoState be= ginWhat, ArrayOption option) Private::AggregateInfo &aggregateInfo =3D d->m_aggregateStack.= back(); if (aggregateInfo.aggregateType =3D=3D beginWhat) { // No writes to the array or dict may have occurred yet - if (d->m_signature.length =3D=3D aggregateInfo.arr.contain= edTypeBegin) { + + if (d->m_signaturePosition =3D=3D aggregateInfo.arr.contai= nedTypeBegin) { // Fix up state as if beginArray/Dict() had been calle= d with WriteTypesOfEmptyArray // in the first place. After that small fixup we're do= ne and return. // The code is a slightly modified version of code bel= ow under: if (isEmpty) { if (!d->m_nilArrayNesting) { d->m_nilArrayNesting =3D 1; - d->m_dataElementsCountBeforeNilArray =3D d->m_elem= ents.size() + 1; + d->m_dataElementsCountBeforeNilArray =3D d->m_elem= ents.size() + 1; // as below + // Now correct for the elements already added in a= dvanceState() with BeginArray / BeginDict + d->m_dataElementsCountBeforeNilArray -=3D (beginWh= at =3D=3D BeginDict) ? 2 : 1; d->m_dataPositionBeforeNilArray =3D d->m_dataPosit= ion; } else { // The array may be implicitly nil (so our poor AP= I client doesn't notice) because diff --git a/tests/serialization/tst_arguments.cpp b/tests/serialization/ts= t_arguments.cpp index 3ac3110..c8a04fd 100644 --- a/tests/serialization/tst_arguments.cpp +++ b/tests/serialization/tst_arguments.cpp @@ -1521,6 +1521,27 @@ static void test_emptyArrayAndDict() doRoundtrip(arg, false); } } + for (int i =3D 0; i < 4; i++) { + // Test RestartEmptyArrayToWriteTypes and writing an empty array i= nside the >1st iteration of another array + Arguments::Writer writer; + writer.beginArray((i & 2) ? Arguments::Writer::WriteTypesOfEmptyAr= ray : Arguments::Writer::NonEmptyArray); + writer.beginArray(Arguments::Writer::NonEmptyArray); // don't = care, the logic error is only in the second iteration + writer.writeString(cstring("a")); + writer.endArray(); + if (i & 1) { + writer.beginArray(Arguments::Writer::WriteTypesOfEmptyArra= y); + } else { + writer.beginArray(Arguments::Writer::NonEmptyArray); + writer.beginArray(Arguments::Writer::RestartEmptyArrayToWr= iteTypes); + } + writer.writeString(cstring("a")); + writer.endArray(); + writer.endArray(); + TEST(writer.state() !=3D Arguments::InvalidData); + Arguments arg =3D writer.finish(); + TEST(writer.state() =3D=3D Arguments::Finished); + doRoundtrip(arg, false); + } { for (int i =3D 0; i <=3D 32; i++) { Arguments::Writer writer; @@ -1576,9 +1597,14 @@ static void test_emptyArrayAndDict() writer.writeString(cstring("a")); writer.beginVariant(); writer.endVariant(); - TEST(writer.state() !=3D Arguments::InvalidData); writer.writeString(cstring("a")); - TEST(writer.state() =3D=3D Arguments::InvalidData); + writer.beginVariant(); + writer.endVariant(); + writer.endDict(); + TEST(writer.state() !=3D Arguments::InvalidData); + Arguments arg =3D writer.finish(); + TEST(writer.state() =3D=3D Arguments::Finished); + doRoundtrip(arg, false); } { Arguments::Writer writer; @@ -1595,6 +1621,31 @@ static void test_emptyArrayAndDict() TEST(writer.state() =3D=3D Arguments::Finished); doRoundtrip(arg, false); } + for (int i =3D 0; i < 4; i++) { + // Test RestartEmptyArrayToWriteTypes and writing an empty dict in= side the >1st iteration of another dict + Arguments::Writer writer; + writer.beginDict((i & 2) ? Arguments::Writer::WriteTypesOfEmptyArr= ay : Arguments::Writer::NonEmptyArray); + writer.writeString(cstring("a")); + writer.beginDict(Arguments::Writer::NonEmptyArray); // don't c= are, the logic error is only in the second iteration + writer.writeString(cstring("a")); + writer.writeInt32(1234); + writer.endDict(); + writer.writeString(cstring("a")); + if (i & 1) { + writer.beginDict(Arguments::Writer::WriteTypesOfEmptyArray= ); + } else { + writer.beginDict(Arguments::Writer::NonEmptyArray); + writer.beginDict(Arguments::Writer::RestartEmptyArrayToWri= teTypes); + } + writer.writeString(cstring("a")); + writer.writeInt32(1234); + writer.endDict(); + writer.endDict(); + TEST(writer.state() !=3D Arguments::InvalidData); + Arguments arg =3D writer.finish(); + TEST(writer.state() =3D=3D Arguments::Finished); + doRoundtrip(arg, false); + } { for (int i =3D 0; i <=3D 32; i++) { Arguments::Writer writer; @@ -1619,7 +1670,6 @@ static void test_emptyArrayAndDict() doRoundtrip(arg, false); } } - } = // TODO: test where we compare data and signature lengths of all combinati= ons of zero/nonzero array diff --git a/util/error.h b/util/error.h index 247da72..cba439f 100644 --- a/util/error.h +++ b/util/error.h @@ -71,7 +71,6 @@ public: CannotEndArrayHere, CannotEndArrayOrDictHere, TooFewTypesInArrayOrDict, - ExtraIterationInEmptyArray, InvalidStateToRestartEmptyArray, InvalidKeyTypeInDict, GreaterTwoTypesInDict,