[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: [dferry] /: Arguments::Reader/Writer: add isInsideEmptyArray() and currentSignature().
From: Andreas Hartmetz <ahartmetz () gmail ! com>
Date: 2016-10-17 23:32:34
Message-ID: E1bwHOU-0000V1-CR () code ! kde ! org
[Download RAW message or body]
Git commit e5ebd40e0ebf7123d6106057b332ed29d38c5509 by Andreas Hartmetz.
Committed on 17/10/2016 at 23:22.
Pushed by ahartmetz into branch 'master'.
Arguments::Reader/Writer: add isInsideEmptyArray() and currentSignature().
They will (probably) be needed in QtDBus and they are somewhat useful
in any case.
M +21 -22 applications/analyzer/argumentsmodel.cpp
M +83 -26 serialization/arguments.cpp
M +4 -0 serialization/arguments.h
M +8 -15 tests/serialization/tst_arguments.cpp
http://commits.kde.org/dferry/e5ebd40e0ebf7123d6106057b332ed29d38c5509
diff --git a/applications/analyzer/argumentsmodel.cpp b/applications/analyzer/argumentsmodel.cpp
index 0dc97b6..4e8dbb4 100644
--- a/applications/analyzer/argumentsmodel.cpp
+++ b/applications/analyzer/argumentsmodel.cpp
@@ -88,7 +88,8 @@ QAbstractItemModel* createArgumentsModel(Message *message)
}
bool isDone = false;
- int emptyNesting = 0;
+ // Cache it, don't call Reader::isInsideEmptyArray() on every data element.
+ bool inEmptyArray = false;
while (!isDone) {
switch(reader.state()) {
@@ -112,63 +113,61 @@ QAbstractItemModel* createArgumentsModel(Message *message)
parent = ascend(parent, model);
break;
case Arguments::BeginArray: {
- const bool hasData = reader.beginArray(Arguments::Reader::ReadTypesOnlyIfEmpty);
- parent = descend(parent, hasData ? "Array" : "Array (no elements, showing just types)");
- emptyNesting += hasData ? 0 : 1;
+ inEmptyArray = !reader.beginArray(Arguments::Reader::ReadTypesOnlyIfEmpty);
+ parent = descend(parent, inEmptyArray ? "Array (no elements, showing just types)" : "Array");
break; }
case Arguments::EndArray:
reader.endArray();
+ inEmptyArray = reader.isInsideEmptyArray();
parent = ascend(parent, model);
- emptyNesting = qMax(emptyNesting - 1, 0);
break;
case Arguments::BeginDict: {
- const bool hasData = reader.beginDict(Arguments::Reader::ReadTypesOnlyIfEmpty);
- parent = descend(parent, hasData ? "Dict" : "Dict (no elements, showing just types)");
- emptyNesting += hasData ? 0 : 1;
+ inEmptyArray = !reader.beginDict(Arguments::Reader::ReadTypesOnlyIfEmpty);
+ parent = descend(parent, inEmptyArray ? "Dict (no elements, showing just types)" : "Dict");
break; }
case Arguments::EndDict:
reader.endDict();
+ inEmptyArray = reader.isInsideEmptyArray();
parent = ascend(parent, model);
- emptyNesting = qMax(emptyNesting - 1, 0);
break;
case Arguments::Byte:
- addKeyValue(parent, "byte", emptyNesting, reader.readByte());
+ addKeyValue(parent, "byte", inEmptyArray, reader.readByte());
break;
case Arguments::Boolean:
- addKeyValue(parent, "boolean", emptyNesting, reader.readBoolean());
+ addKeyValue(parent, "boolean", inEmptyArray, reader.readBoolean());
break;
case Arguments::Int16:
- addKeyValue(parent, "int16", emptyNesting, reader.readInt16());
+ addKeyValue(parent, "int16", inEmptyArray, reader.readInt16());
break;
case Arguments::Uint16:
- addKeyValue(parent, "uint16", emptyNesting, reader.readUint16());
+ addKeyValue(parent, "uint16", inEmptyArray, reader.readUint16());
break;
case Arguments::Int32:
- addKeyValue(parent, "int32", emptyNesting, reader.readInt32());
+ addKeyValue(parent, "int32", inEmptyArray, reader.readInt32());
break;
case Arguments::Uint32:
- addKeyValue(parent, "uint32", emptyNesting, reader.readUint32());
+ addKeyValue(parent, "uint32", inEmptyArray, reader.readUint32());
break;
case Arguments::Int64:
- addKeyValue(parent, "int64", emptyNesting, reader.readInt64());
+ addKeyValue(parent, "int64", inEmptyArray, reader.readInt64());
break;
case Arguments::Uint64:
- addKeyValue(parent, "uint64", emptyNesting, reader.readUint64());
+ addKeyValue(parent, "uint64", inEmptyArray, reader.readUint64());
break;
case Arguments::Double:
- addKeyValue(parent, "double", emptyNesting, reader.readDouble());
+ addKeyValue(parent, "double", inEmptyArray, reader.readDouble());
break;
case Arguments::String:
- addKeyValue(parent, "string", emptyNesting, reader.readString().ptr);
+ addKeyValue(parent, "string", inEmptyArray, reader.readString().ptr);
break;
case Arguments::ObjectPath:
- addKeyValue(parent, "object path", emptyNesting, reader.readObjectPath().ptr);
+ addKeyValue(parent, "object path", inEmptyArray, reader.readObjectPath().ptr);
break;
case Arguments::Signature:
- addKeyValue(parent, "type signature", emptyNesting, reader.readSignature().ptr);
+ addKeyValue(parent, "type signature", inEmptyArray, reader.readSignature().ptr);
break;
case Arguments::UnixFd:
- addKeyValue(parent, "file descriptor", emptyNesting, QVariant());
+ addKeyValue(parent, "file descriptor", inEmptyArray, QVariant());
break;
case Arguments::InvalidData:
case Arguments::NeedMoreData:
diff --git a/serialization/arguments.cpp b/serialization/arguments.cpp
index 0e7c5a5..6c03f83 100644
--- a/serialization/arguments.cpp
+++ b/serialization/arguments.cpp
@@ -507,7 +507,10 @@ std::string Arguments::prettyPrint() const
std::string nestingPrefix;
bool isDone = false;
- int emptyNesting = 0;
+
+ // Cache it, don't call Reader::isInsideEmptyArray() on every data element. This isn't really
+ // a big deal for performance here, but in other situations it is, so set a good example :)
+ bool inEmptyArray = false;
while (!isDone) {
// HACK use nestingPrefix to determine when we're switching from key to value - this can be done
@@ -548,27 +551,26 @@ std::string Arguments::prettyPrint() const
const std::pair<Arguments::IoState, chunk> bytes = reader.readPrimitiveArray();
assert(bytes.first == Arguments::Byte);
assert(bytes.second.length > 0);
+ inEmptyArray = reader.isInsideEmptyArray(); // Maybe not necessary, but safe
ret << nestingPrefix << "array of bytes [ " << uint(bytes.second.ptr[0]);
for (uint32 i = 1; i < bytes.second.length; i++) {
ret << ", " << uint(bytes.second.ptr[i]);
}
ret << " ]\n";
} else {
- const bool hasData = reader.beginArray(Arguments::Reader::ReadTypesOnlyIfEmpty);
- emptyNesting += hasData ? 0 : 1;
+ inEmptyArray = !reader.beginArray(Arguments::Reader::ReadTypesOnlyIfEmpty);
ret << nestingPrefix << "begin array\n";
nestingPrefix += "[ ";
}
break;
case Arguments::EndArray:
reader.endArray();
- emptyNesting = std::max(emptyNesting - 1, 0);
+ inEmptyArray = reader.isInsideEmptyArray();
nestingPrefix.resize(nestingPrefix.size() - 2);
ret << nestingPrefix << "end array\n";
break;
case Arguments::BeginDict: {
- const bool hasData = reader.beginDict(Arguments::Reader::ReadTypesOnlyIfEmpty);
- emptyNesting += hasData ? 0 : 1;
+ inEmptyArray = !reader.beginDict(Arguments::Reader::ReadTypesOnlyIfEmpty);
ret << nestingPrefix << "begin dict\n";
nestingPrefix += "{K ";
break; }
@@ -583,14 +585,14 @@ std::string Arguments::prettyPrint() const
#endif
case Arguments::EndDict:
reader.endDict();
- emptyNesting = std::max(emptyNesting - 1, 0);
+ inEmptyArray = reader.isInsideEmptyArray();
nestingPrefix.resize(nestingPrefix.size() - strlen("{V "));
ret << nestingPrefix << "end dict\n";
break;
case Arguments::Boolean: {
bool b = reader.readBoolean();
ret << nestingPrefix << "bool: ";
- if (emptyNesting) {
+ if (inEmptyArray) {
ret << "<nil>";
} else {
ret << (b ? "true" : "false");
@@ -598,37 +600,37 @@ std::string Arguments::prettyPrint() const
ret << '\n';
break; }
case Arguments::Byte:
- printMaybeNil(&ret, nestingPrefix, emptyNesting, int(reader.readByte()), "byte");
+ printMaybeNil(&ret, nestingPrefix, inEmptyArray, int(reader.readByte()), "byte");
break;
case Arguments::Int16:
- printMaybeNil(&ret, nestingPrefix, emptyNesting, reader.readInt16(), "int16");
+ printMaybeNil(&ret, nestingPrefix, inEmptyArray, reader.readInt16(), "int16");
break;
case Arguments::Uint16:
- printMaybeNil(&ret, nestingPrefix, emptyNesting, reader.readUint16(), "uint16");
+ printMaybeNil(&ret, nestingPrefix, inEmptyArray, reader.readUint16(), "uint16");
break;
case Arguments::Int32:
- printMaybeNil(&ret, nestingPrefix, emptyNesting, reader.readInt32(), "int32");
+ printMaybeNil(&ret, nestingPrefix, inEmptyArray, reader.readInt32(), "int32");
break;
case Arguments::Uint32:
- printMaybeNil(&ret, nestingPrefix, emptyNesting, reader.readUint32(), "uint32");
+ printMaybeNil(&ret, nestingPrefix, inEmptyArray, reader.readUint32(), "uint32");
break;
case Arguments::Int64:
- printMaybeNil(&ret, nestingPrefix, emptyNesting, reader.readInt64(), "int64");
+ printMaybeNil(&ret, nestingPrefix, inEmptyArray, reader.readInt64(), "int64");
break;
case Arguments::Uint64:
- printMaybeNil(&ret, nestingPrefix, emptyNesting, reader.readUint64(), "uint64");
+ printMaybeNil(&ret, nestingPrefix, inEmptyArray, reader.readUint64(), "uint64");
break;
case Arguments::Double:
- printMaybeNil(&ret, nestingPrefix, emptyNesting, reader.readDouble(), "double");
+ printMaybeNil(&ret, nestingPrefix, inEmptyArray, reader.readDouble(), "double");
break;
case Arguments::String:
- printMaybeNil(&ret, nestingPrefix, emptyNesting, reader.readString(), "string");
+ printMaybeNil(&ret, nestingPrefix, inEmptyArray, reader.readString(), "string");
break;
case Arguments::ObjectPath:
- printMaybeNil(&ret, nestingPrefix, emptyNesting, reader.readObjectPath(), "object path");
+ printMaybeNil(&ret, nestingPrefix, inEmptyArray, reader.readObjectPath(), "object path");
break;
case Arguments::Signature:
- printMaybeNil(&ret, nestingPrefix, emptyNesting, reader.readSignature(), "type signature");
+ printMaybeNil(&ret, nestingPrefix, inEmptyArray, reader.readSignature(), "type signature");
break;
case Arguments::UnixFd:
// TODO
@@ -940,6 +942,36 @@ cstring Arguments::Reader::stateString() const
return printableState(m_state);
}
+bool Arguments::Reader::isInsideEmptyArray() const
+{
+ if (d->m_nilArrayNesting > 1) {
+ return true;
+ } else if (d->m_nilArrayNesting == 1) {
+ // Special case 1: in state BeginArray/Dict, when we've just entered the nil array according
+ // to our internal state, the API user cannot know that yet -> return false;
+ // Since this is easily done here, don't complicate the rest of the code with a hacky solution.
+ if (m_state == BeginArray || m_state == BeginDict) {
+ return false;
+ }
+ // Special case 2: in state EndArray/Dict, when we've just left the nil array according
+ // to our internal state, advanceState() and endArray/Dict() make sure that
+ // d->m_nilArrayNesting is only decremented after endArray/Dict() has been called.
+ // Nothing to do for us here, but this is the point to explain it.
+ return true;
+ } else {
+ assert(d->m_nilArrayNesting == 0);
+ return false;
+ }
+}
+
+cstring Arguments::Reader::currentSignature() const
+{
+ // ### not sure if determining length like that is the best way, should probably be as similar to
+ // libdbus-1 as possible (unless we can do much better).
+ return cstring(d->m_signature.ptr,
+ std::max(uint32(0), std::min(d->m_signature.length, d->m_signaturePosition)));
+}
+
void Arguments::Reader::replaceData(chunk data)
{
VALID_IF(data.length >= d->m_dataPosition, Error::ReplacementDataIsShorter);
@@ -1167,20 +1199,24 @@ void Arguments::Reader::advanceState()
const bool isDict = aggregateInfo.aggregateType == BeginDict;
if (d->m_signaturePosition > aggregateInfo.arr.containedTypeBegin + (isDict ? 1 : 0)) {
const Private::ArrayInfo &arrayInfo = aggregateInfo.arr;
- bool moreElements = false;
+ bool moreIterations = false;
if (unlikely(d->m_nilArrayNesting)) {
// we're done iterating over it once
// TODO unit-test this
- d->m_nilArrayNesting--;
+ // HACK: decreasing m_nilArrayNesting here would be much cleaner! But from the
+ // user's POV, the nil array has not been left before the call to endArray().
+ // We will set state EndArray / EndDict, so we know that the user "must" call
+ // endArray()/endDict(), so adjust d->m_nilArrayNesting in those methods.
+ //d->m_nilArrayNesting--;
} else if (d->m_dataPosition < arrayInfo.dataEnd) {
if (isDict) {
d->m_dataPosition = align(d->m_dataPosition, 8); // align to dict entry
}
// rewind to start of contained type and read the type info there
d->m_signaturePosition = arrayInfo.containedTypeBegin;
- moreElements = true;
+ moreIterations = true;
}
- if (!moreElements) {
+ if (!moreIterations) {
// TODO check that final data position is where it should be according to the
// serialized array length
m_state = isDict ? EndDict : EndArray;
@@ -1326,7 +1362,8 @@ void Arguments::Reader::advanceState()
VALID_IF(d->m_nesting.beginArray(), Error::MalformedMessageData);
if (firstElementTy.state() == BeginDict) {
d->m_signaturePosition++;
- // TODO check whether the first type is a primitive or string type!
+ // TODO check whether the first type is a primitive or string type! // ### isn't that already
+ // checked for the main signature and / or variants, though?
// only closed at end of dict - there is no observable difference for clients
VALID_IF(d->m_nesting.beginParen(), Error::MalformedMessageData);
}
@@ -1462,7 +1499,11 @@ void Arguments::Reader::skipArray()
void Arguments::Reader::endArray()
{
- advanceStateFrom(EndArray);
+ VALID_IF(m_state == EndArray, Error::ReadWrongType);
+ if (unlikely(d->m_nilArrayNesting)) {
+ d->m_nilArrayNesting--;
+ }
+ advanceState();
}
static bool isAligned(uint32 value, uint32 alignment)
@@ -1562,7 +1603,11 @@ void Arguments::Reader::skipDict()
void Arguments::Reader::endDict()
{
- advanceStateFrom(EndDict);
+ VALID_IF(m_state == EndDict, Error::ReadWrongType);
+ if (unlikely(d->m_nilArrayNesting)) {
+ d->m_nilArrayNesting--;
+ }
+ advanceState();
}
void Arguments::Reader::beginStruct()
@@ -1853,6 +1898,18 @@ cstring Arguments::Writer::stateString() const
return printableState(m_state);
}
+bool Arguments::Writer::isInsideEmptyArray() const
+{
+ return d->m_nilArrayNesting > 0;
+}
+
+cstring Arguments::Writer::currentSignature() const
+{
+ // ### see comment in Reader::currentSignature
+ return cstring(d->m_signature.ptr,
+ std::max(uint32(0), std::min(d->m_signature.length, d->m_signaturePosition)));
+}
+
void Arguments::Writer::doWritePrimitiveType(uint32 alignAndSize)
{
d->m_dataPosition = align(d->m_dataPosition, alignAndSize);
diff --git a/serialization/arguments.h b/serialization/arguments.h
index d557c8a..cf7719d 100644
--- a/serialization/arguments.h
+++ b/serialization/arguments.h
@@ -169,6 +169,8 @@ public:
IoState state() const { return m_state; }
cstring stateString() const;
+ bool isInsideEmptyArray() const;
+ cstring currentSignature() const; // current signature, either main signature or current variant
// HACK call this in NeedMoreData state when more data has been added; this replaces m_data
// WARNING: calling replaceData() invalidates copies (if any) of this Reader
void replaceData(chunk data);
@@ -288,6 +290,8 @@ public:
IoState state() const { return m_state; }
cstring stateString() const;
+ bool isInsideEmptyArray() const;
+ cstring currentSignature() const; // current signature, either main signature or current variant
enum ArrayOption
{
diff --git a/tests/serialization/tst_arguments.cpp b/tests/serialization/tst_arguments.cpp
index 3546d04..39dce0b 100644
--- a/tests/serialization/tst_arguments.cpp
+++ b/tests/serialization/tst_arguments.cpp
@@ -70,7 +70,7 @@ static bool stringsEqual(cstring s1, cstring s2)
// 3) skips nil arrays at and below nil array nesting level m_skipNilArraysFromLevel with m_skippingReader
// It even skips aggregates inside nil arrays as 2) + 3) imply.
// It checks:
-// a) where nothing is skipped that the aggregate structure and data read is the same
+// a) where nothing is skipped, that the aggregate structure and data read are the same
class SkipChecker
{
public:
@@ -284,7 +284,7 @@ static void testReadWithSkip(const Arguments &arg, bool debugPrint)
// When using this to iterate over the reader, it will make an exact copy using the Writer.
// You need to do something only in states where something special should happen.
-static void defaultReadToWrite(Arguments::Reader *reader, Arguments::Writer *writer, uint32 *emptyNesting)
+static void defaultReadToWrite(Arguments::Reader *reader, Arguments::Writer *writer)
{
switch(reader->state()) {
case Arguments::BeginStruct:
@@ -307,23 +307,19 @@ static void defaultReadToWrite(Arguments::Reader *reader, Arguments::Writer *wri
const bool hasData = reader->beginArray(Arguments::Reader::ReadTypesOnlyIfEmpty);
writer->beginArray(hasData ? Arguments::Writer::NonEmptyArray
: Arguments::Writer::WriteTypesOfEmptyArray);
- *emptyNesting += hasData ? 0 : 1;
break; }
case Arguments::EndArray:
reader->endArray();
writer->endArray();
- *emptyNesting = std::max(*emptyNesting - 1, 0u);
break;
case Arguments::BeginDict: {
const bool hasData = reader->beginDict(Arguments::Reader::ReadTypesOnlyIfEmpty);
writer->beginDict(hasData ? Arguments::Writer::NonEmptyArray
: Arguments::Writer::WriteTypesOfEmptyArray);
- *emptyNesting += hasData ? 0 : 1;
break; }
case Arguments::EndDict:
reader->endDict();
writer->endDict();
- *emptyNesting = std::max(*emptyNesting - 1, 0u);
break;
case Arguments::Byte:
writer->writeByte(reader->readByte());
@@ -354,7 +350,7 @@ static void defaultReadToWrite(Arguments::Reader *reader, Arguments::Writer *wri
break;
case Arguments::String: {
cstring s = reader->readString();
- if (*emptyNesting) {
+ if (reader->isInsideEmptyArray()) {
s = cstring("");
} else {
TEST(Arguments::isStringValid(s));
@@ -363,7 +359,7 @@ static void defaultReadToWrite(Arguments::Reader *reader, Arguments::Writer *wri
break; }
case Arguments::ObjectPath: {
cstring objectPath = reader->readObjectPath();
- if (*emptyNesting) {
+ if (reader->isInsideEmptyArray()) {
objectPath = cstring("/");
} else {
TEST(Arguments::isObjectPathValid(objectPath));
@@ -372,7 +368,7 @@ static void defaultReadToWrite(Arguments::Reader *reader, Arguments::Writer *wri
break; }
case Arguments::Signature: {
cstring signature = reader->readSignature();
- if (*emptyNesting) {
+ if (reader->isInsideEmptyArray()) {
signature = cstring("");
} else {
TEST(Arguments::isSignatureValid(signature));
@@ -428,7 +424,6 @@ static void doRoundtripWithShortReads(const Arguments &original, uint32 dataIncr
chunk data = original.data();
chunk shortData;
bool isDone = false;
- uint32 emptyNesting = 0;
while (!isDone) {
TEST(writer.state() != Arguments::InvalidData);
@@ -460,7 +455,7 @@ static void doRoundtripWithShortReads(const Arguments &original, uint32 dataIncr
reader.replaceData(shortData);
break; }
default:
- defaultReadToWrite(&reader, &writer, &emptyNesting);
+ defaultReadToWrite(&reader, &writer);
break;
}
}
@@ -478,7 +473,6 @@ static void doRoundtripWithReaderCopy(const Arguments &original, uint32 dataIncr
Arguments::Writer writer;
bool isDone = false;
- uint32 emptyNesting = 0;
uint32 i = 0;
while (!isDone) {
@@ -496,7 +490,7 @@ static void doRoundtripWithReaderCopy(const Arguments &original, uint32 dataIncr
isDone = true;
break;
default:
- defaultReadToWrite(reader, &writer, &emptyNesting);
+ defaultReadToWrite(reader, &writer);
break;
}
}
@@ -512,7 +506,6 @@ static void doRoundtripWithWriterCopy(const Arguments &original, uint32 dataIncr
Arguments::Writer *writer = new Arguments::Writer;
bool isDone = false;
- uint32 emptyNesting = 0;
uint32 i = 0;
while (!isDone) {
@@ -530,7 +523,7 @@ static void doRoundtripWithWriterCopy(const Arguments &original, uint32 dataIncr
isDone = true;
break;
default:
- defaultReadToWrite(&reader, writer, &emptyNesting);
+ defaultReadToWrite(&reader, writer);
break;
}
}
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic