[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