[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [dferry] serialization: Extend and publicize skipCurrentAggregate to skipCurrentElement.
From:       Andreas Hartmetz <ahartmetz () gmail ! com>
Date:       2016-11-28 17:23:10
Message-ID: E1cBPe2-00057I-Hf () code ! kde ! org
[Download RAW message or body]

Git commit aad29311ca3e6062e17255975138d26c2d554e42 by Andreas Hartmetz.
Committed on 28/11/2016 at 17:20.
Pushed by ahartmetz into branch 'master'.

Extend and publicize skipCurrentAggregate to skipCurrentElement.

This is needed somewhere in QtDBus - would be nice to have some tests
here but low priority for now.

M  +18   -13   serialization/arguments.cpp
M  +3    -1    serialization/arguments.h

https://commits.kde.org/dferry/aad29311ca3e6062e17255975138d26c2d554e42

diff --git a/serialization/arguments.cpp b/serialization/arguments.cpp
index 1fc78f5..44d5033 100644
--- a/serialization/arguments.cpp
+++ b/serialization/arguments.cpp
@@ -1703,7 +1703,7 @@ void Arguments::Reader::skipStruct()
         m_state = InvalidData;
         d->m_error.setCode(Error::ReadWrongType);
     } else {
-        skipCurrentAggregate();
+        skipCurrentElement();
     }
 }
 
@@ -1739,7 +1739,7 @@ void Arguments::Reader::skipVariant()
         m_state = InvalidData;
         d->m_error.setCode(Error::ReadWrongType);
     } else {
-        skipCurrentAggregate();
+        skipCurrentElement();
     }
 }
 
@@ -1758,12 +1758,11 @@ void Arguments::Reader::endVariant()
     advanceState();
 }
 
-void Arguments::Reader::skipCurrentAggregate()
+void Arguments::Reader::skipCurrentElement()
 {
-    // ### the serialized format has no quick way to do this and it makes little sense to implement
-    //     a "fast path" just to do this slow thing a little faster, so just use public API!
+    // ### We could implement a skipping fast path for more aggregates, but it would be a lot of work, so
+    //     until it's proven to be a problem, just reuse what we have.
 
-    assert(m_state == BeginStruct || m_state == BeginVariant);
 #ifndef NDEBUG
     Arguments::IoState stateOnEntry = m_state;
 #endif
@@ -1773,8 +1772,9 @@ void Arguments::Reader::skipCurrentAggregate()
     while (!isDone) {
         switch(state()) {
         case Arguments::Finished:
-            // We should never get here because we should have already quit when leaving the main aggregate
-            // that this was called on, i.e. in state EndStruct / EndVariant, which come before Finished
+            // Okay, that's a bit weird. I guess the graceful way to handle it is do nothing in release
+            // mode, and explode in debug mode in order to warn the API client.
+            // (We could use a warning message facility here, make one?)
             assert(false);
             isDone = true;
             break;
@@ -1787,7 +1787,6 @@ void Arguments::Reader::skipCurrentAggregate()
             nestingLevel--;
             if (!nestingLevel) {
                 assert(stateOnEntry == BeginStruct);
-                isDone = true;
             }
             break;
         case Arguments::BeginVariant:
@@ -1799,20 +1798,23 @@ void Arguments::Reader::skipCurrentAggregate()
             nestingLevel--;
             if (!nestingLevel) {
                 assert(stateOnEntry == BeginVariant);
-                isDone = true;
             }
             break;
         case Arguments::BeginArray:
             skipArray();
             break;
         case Arguments::EndArray:
-            assert(false); // can't happen because we skip all arrays
+            assert(stateOnEntry == EndArray); // only way this can happen - we gracefully skip EndArray
+                                              // and DON'T decrease nestingLevel b/c it would go negative.
+            endArray();
             break;
         case Arguments::BeginDict:
             skipDict();
             break;
         case Arguments::EndDict:
-            assert(false); // can't happen because we skip all dicts
+            assert(stateOnEntry == EndDict); // only way this can happen - we gracefully "skip" EndDict
+                                             // and DON'T decrease nestingLevel b/c it would go negative.
+            endDict();
             break;
         case Arguments::Boolean:
             readBoolean();
@@ -1851,7 +1853,7 @@ void Arguments::Reader::skipCurrentAggregate()
             readSignature();
             break;
         case Arguments::UnixFd:
-            // TODO
+            readUnixFd();
             break;
         case Arguments::NeedMoreData:
             // TODO handle this properly: rewind the state to before the aggregate - or get fancy and support
@@ -1865,6 +1867,9 @@ void Arguments::Reader::skipCurrentAggregate()
             isDone = true;
             break;
         }
+        if (!nestingLevel) {
+            isDone = true;
+        }
     }
 }
 
diff --git a/serialization/arguments.h b/serialization/arguments.h
index 781baed..a4b28c0 100644
--- a/serialization/arguments.h
+++ b/serialization/arguments.h
@@ -240,6 +240,9 @@ public:
         cstring readSignature() { cstring ret(m_u.String.ptr, m_u.String.length); advanceState(); return ret; }
         uint32 readUnixFd() { uint32 ret = m_u.Uint32; advanceState(); return ret; }
 
+        void skipCurrentElement(); // works on single values and Begin... states. In the Begin... states,
+                                   // skips the whole aggregate.
+
         // Returns primitive type and the raw array data if in BeginArray state of an array containing only a
         // primitive type. You must copy the data before destroying the Reader or changing its backing store
         // with replaceData().
@@ -265,7 +268,6 @@ public:
         void beginArrayOrDict(bool isDict, EmptyArrayOption option);
         void skipArrayOrDictSignature(bool isDict);
         void skipArrayOrDict(bool isDict);
-        void skipCurrentAggregate();
 
         Private *d;
 
[prev in list] [next in list] [prev in thread] [next in thread]