[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: [dferry] /: Changes to empty array handling, from usage experience.
From: Andreas Hartmetz <ahartmetz () gmail ! com>
Date: 2016-11-05 15:58:17
Message-ID: E1c33MH-00034H-HN () code ! kde ! org
[Download RAW message or body]
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 \
signatureFragment, IoState newState }
d->m_signature.length += signatureFragment.length;
} else {
- VALID_IF(likely(!d->m_nilArrayNesting), Error::ExtraIterationInEmptyArray);
+ // Do not try to prevent several iterations through a nil array. Two \
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 through a nil array + // several times. Which is well possible. We \
don't bother with that because... + // - As a QtDBus unittest illustrates, \
somebody may choose to serialize a fixed length + // series of data elements \
as an array (instead of struct), so that a trivial + // serialization of \
such data just to fill in type information in an outer empty array + // \
would end up iterating through the inner, implicitly empty array several times. + \
// All in all it is just not much of a benefit to be strict, so don't. + \
//VALID_IF(likely(!d->m_nilArrayNesting), Error::ExtraIterationInEmptyArray); +
// signature must match first iteration (of an array/dict)
VALID_IF(d->m_signaturePosition + signatureFragment.length <= \
d->m_signature.length, Error::TypeMismatchInSubsequentArrayIteration);
@@ -2236,13 +2246,16 @@ void Arguments::Writer::beginArrayOrDict(IoState beginWhat, \
ArrayOption option)
Private::AggregateInfo &aggregateInfo = d->m_aggregateStack.back();
if (aggregateInfo.aggregateType == beginWhat) {
// No writes to the array or dict may have occurred yet
- if (d->m_signature.length == aggregateInfo.arr.containedTypeBegin) {
+
+ if (d->m_signaturePosition == aggregateInfo.arr.containedTypeBegin) \
{
// Fix up state as if beginArray/Dict() had been called with \
WriteTypesOfEmptyArray
// in the first place. After that small fixup we're done and \
return.
// The code is a slightly modified version of code below under: \
if (isEmpty) { if (!d->m_nilArrayNesting) {
d->m_nilArrayNesting = 1;
- d->m_dataElementsCountBeforeNilArray = d->m_elements.size() \
+ 1; + d->m_dataElementsCountBeforeNilArray = \
d->m_elements.size() + 1; // as below + // Now correct for the \
elements already added in advanceState() with BeginArray / BeginDict + \
d->m_dataElementsCountBeforeNilArray -= (beginWhat == BeginDict) ? 2 \
: 1;
d->m_dataPositionBeforeNilArray = d->m_dataPosition;
} else {
// The array may be implicitly nil (so our poor API client \
doesn't notice) because
diff --git a/tests/serialization/tst_arguments.cpp \
b/tests/serialization/tst_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 = 0; i < 4; i++) {
+ // Test RestartEmptyArrayToWriteTypes and writing an empty array inside the \
>1st iteration of another array + Arguments::Writer writer;
+ writer.beginArray((i & 2) ? Arguments::Writer::WriteTypesOfEmptyArray : \
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::WriteTypesOfEmptyArray);
+ } else {
+ writer.beginArray(Arguments::Writer::NonEmptyArray);
+ writer.beginArray(Arguments::Writer::RestartEmptyArrayToWriteTypes);
+ }
+ writer.writeString(cstring("a"));
+ writer.endArray();
+ writer.endArray();
+ TEST(writer.state() != Arguments::InvalidData);
+ Arguments arg = writer.finish();
+ TEST(writer.state() == Arguments::Finished);
+ doRoundtrip(arg, false);
+ }
{
for (int i = 0; i <= 32; i++) {
Arguments::Writer writer;
@@ -1576,9 +1597,14 @@ static void test_emptyArrayAndDict()
writer.writeString(cstring("a"));
writer.beginVariant();
writer.endVariant();
- TEST(writer.state() != Arguments::InvalidData);
writer.writeString(cstring("a"));
- TEST(writer.state() == Arguments::InvalidData);
+ writer.beginVariant();
+ writer.endVariant();
+ writer.endDict();
+ TEST(writer.state() != Arguments::InvalidData);
+ Arguments arg = writer.finish();
+ TEST(writer.state() == Arguments::Finished);
+ doRoundtrip(arg, false);
}
{
Arguments::Writer writer;
@@ -1595,6 +1621,31 @@ static void test_emptyArrayAndDict()
TEST(writer.state() == Arguments::Finished);
doRoundtrip(arg, false);
}
+ for (int i = 0; i < 4; i++) {
+ // Test RestartEmptyArrayToWriteTypes and writing an empty dict inside the \
>1st iteration of another dict + Arguments::Writer writer;
+ writer.beginDict((i & 2) ? Arguments::Writer::WriteTypesOfEmptyArray : \
Arguments::Writer::NonEmptyArray); + writer.writeString(cstring("a"));
+ writer.beginDict(Arguments::Writer::NonEmptyArray); // don't care, 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::RestartEmptyArrayToWriteTypes);
+ }
+ writer.writeString(cstring("a"));
+ writer.writeInt32(1234);
+ writer.endDict();
+ writer.endDict();
+ TEST(writer.state() != Arguments::InvalidData);
+ Arguments arg = writer.finish();
+ TEST(writer.state() == Arguments::Finished);
+ doRoundtrip(arg, false);
+ }
{
for (int i = 0; i <= 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 combinations 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,
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic