QCborStreamReader: update to the new TinyCBOR zero-copy string API

Change-Id: Iab119b62106d40fb8499fffd1510abe5d8f2722a
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>

src/3rdparty/tinycbor/src/cbor.h

@@ -166,8 +166,7 @@ typedef enum CborError {
     CborErrorIllegalType,           /* type not allowed here */
     CborErrorIllegalNumber,
     CborErrorIllegalSimpleType,     /* types of value less than 32 encoded in two bytes */
-
+    CborErrorNoMoreStringChunks,
-    CborErrorLastStringChunk,       /* not really an error */
 
     /* parser errors in strict mode parsing only */
     CborErrorUnknownSimpleType = 512,
@@ -291,11 +290,23 @@ enum CborParserGlobalFlags
 
 enum CborParserIteratorFlags
 {
+    /* used for all types, but not during string chunk iteration
+     * (values are static-asserted, don't change) */
     CborIteratorFlag_IntegerValueIs64Bit    = 0x01,
     CborIteratorFlag_IntegerValueTooLarge   = 0x02,
+
+    /* used only for CborIntegerType */
     CborIteratorFlag_NegativeInteger        = 0x04,
+
+    /* used only during string iteration */
+    CborIteratorFlag_BeforeFirstStringChunk = 0x04,
     CborIteratorFlag_IteratingStringChunks  = 0x08,
+
+    /* used for arrays, maps and strings, including during chunk iteration */
     CborIteratorFlag_UnknownLength          = 0x10,
+
+    /* used for maps, but must be kept for all types
+     * (ContainerIsMap value must be CborMapType - CborArrayType) */
     CborIteratorFlag_ContainerIsMap         = 0x20,
     CborIteratorFlag_NextIsMapKey           = 0x40
 };
@@ -496,9 +507,36 @@ CBOR_INLINE_API CborError cbor_value_dup_byte_string(const CborValue *value, uin
     return _cbor_value_dup_string(value, (void **)buffer, buflen, next);
 }
 
+CBOR_PRIVATE_API CborError _cbor_value_get_string_chunk_size(const CborValue *value, size_t *len);
+CBOR_INLINE_API CborError cbor_value_get_string_chunk_size(const CborValue *value, size_t *len)
+{
+    assert(value->flags & CborIteratorFlag_IteratingStringChunks);
+    return _cbor_value_get_string_chunk_size(value, len);
+}
+
+CBOR_INLINE_API bool cbor_value_string_iteration_at_end(const CborValue *value)
+{
+    size_t dummy;
+    return cbor_value_get_string_chunk_size(value, &dummy) == CborErrorNoMoreStringChunks;
+}
+
+CBOR_PRIVATE_API CborError _cbor_value_begin_string_iteration(CborValue *value);
+CBOR_INLINE_API CborError cbor_value_begin_string_iteration(CborValue *value)
+{
+    assert(cbor_value_is_text_string(value) || cbor_value_is_byte_string(value));
+    assert(!(value->flags & CborIteratorFlag_IteratingStringChunks));
+    return _cbor_value_begin_string_iteration(value);
+}
+
+CBOR_PRIVATE_API CborError _cbor_value_finish_string_iteration(CborValue *value);
+CBOR_INLINE_API CborError cbor_value_finish_string_iteration(CborValue *value)
+{
+    assert(cbor_value_string_iteration_at_end(value));
+    return _cbor_value_finish_string_iteration(value);
+}
+
 CBOR_PRIVATE_API CborError _cbor_value_get_string_chunk(const CborValue *value, const void **bufferptr,
                                                         size_t *len, CborValue *next);
-CBOR_API CborError cbor_value_get_string_chunk_size(CborValue *value, size_t *len);
 CBOR_INLINE_API CborError cbor_value_get_text_string_chunk(const CborValue *value, const char **bufferptr,
                                                            size_t *len, CborValue *next)
 {

src/3rdparty/tinycbor/src/cborerrorstrings.c

@@ -119,8 +119,8 @@ const char *cbor_error_string(CborError error)
     case CborErrorIllegalSimpleType:
         return _("illegal encoding of simple type smaller than 32");
 
-    case CborErrorLastStringChunk:
+    case CborErrorNoMoreStringChunks:
-        return _("no size available: that was the last string chunk");
+        return _("no more byte or text strings available");
 
     case CborErrorUnknownSimpleType:
         return _("unknown simple type");

src/3rdparty/tinycbor/src/cborinternal_p.h

@@ -90,8 +90,6 @@ enum {
     BreakByte               = (unsigned)Break | (SimpleTypesType << MajorTypeShift)
 };
 
-CBOR_INTERNAL_API CborError CBOR_INTERNAL_API_CC _cbor_value_prepare_string_iteration(CborValue *it);
-
 static inline void copy_current_position(CborValue *dst, const CborValue *src)
 {
     // This "if" is here for pedantry only: the two branches should perform

src/3rdparty/tinycbor/src/cborparser.c

@@ -963,60 +963,45 @@ CborError cbor_value_calculate_string_length(const CborValue *value, size_t *len
     return _cbor_value_copy_string(value, NULL, len, NULL);
 }
 
-static inline void prepare_string_iteration(CborValue *it)
+CborError _cbor_value_begin_string_iteration(CborValue *it)
 {
+    it->flags |= CborIteratorFlag_IteratingStringChunks |
+            CborIteratorFlag_BeforeFirstStringChunk;
     if (!cbor_value_is_length_known(it)) {
         /* chunked string: we're before the first chunk;
          * advance to the first chunk */
         advance_bytes(it, 1);
-        it->flags |= CborIteratorFlag_IteratingStringChunks;
-    }
     }
 
-CborError CBOR_INTERNAL_API_CC _cbor_value_prepare_string_iteration(CborValue *it)
-{
-    cbor_assert((it->flags & CborIteratorFlag_IteratingStringChunks) == 0);
-    prepare_string_iteration(it);
-
-    /* are we at the end? */
-    if (!can_read_bytes(it, 1))
-        return CborErrorUnexpectedEOF;
     return CborNoError;
 }
 
-static CborError get_string_chunk_size(CborValue *it, size_t *offset, size_t *len)
+CborError _cbor_value_finish_string_iteration(CborValue *it)
 {
-    /* Possible states:
+    if (!cbor_value_is_length_known(it))
-     * length known | iterating | meaning
+        advance_bytes(it, 1);       /* skip the Break */
-     *     no       |    no     | before the first chunk of a chunked string
+
-     *     yes      |    no     | at a non-chunked string
+    return preparse_next_value(it);
-     *     no       |    yes    | second or later chunk
-     *     yes      |    yes    | after a non-chunked string
-     */
-    if (it->flags & CborIteratorFlag_IteratingStringChunks) {
-        /* already iterating */
-        if (cbor_value_is_length_known(it)) {
-            /* if the length was known, it wasn't chunked, so finish iteration */
-            *len = 0;
-            return CborErrorLastStringChunk;
-        }
-    } else {
-        prepare_string_iteration(it);
 }
 
-    /* are we at the end? */
+static CborError get_string_chunk_size(const CborValue *it, size_t *offset, size_t *len)
+{
     uint8_t descriptor;
+    size_t bytesNeeded = 1;
+
+    if (cbor_value_is_length_known(it) && (it->flags & CborIteratorFlag_BeforeFirstStringChunk) == 0)
+        return CborErrorNoMoreStringChunks;
+
+    /* are we at the end? */
     if (!read_bytes(it, &descriptor, 0, 1))
         return CborErrorUnexpectedEOF;
 
     if (descriptor == BreakByte)
-        return CborErrorLastStringChunk;
+        return CborErrorNoMoreStringChunks;
     if ((descriptor & MajorTypeMask) != it->type)
         return CborErrorIllegalType;
 
     /* find the string length */
-    size_t bytesNeeded = 1;
-
     descriptor &= SmallValueMask;
     if (descriptor < Value8Bit) {
         *len = descriptor;
@@ -1047,45 +1032,33 @@ static CborError get_string_chunk_size(CborValue *it, size_t *offset, size_t *le
         ++bytesNeeded;
     }
 
-    if (*len != (size_t)*len)
-        return CborErrorDataTooLarge;
-
     *offset = bytesNeeded;
     return CborNoError;
 }
 
+CborError _cbor_value_get_string_chunk_size(const CborValue *value, size_t *len)
+{
+    size_t offset;
+    return get_string_chunk_size(value, &offset, len);
+}
+
 static CborError get_string_chunk(CborValue *it, const void **bufferptr, size_t *len)
 {
     size_t offset;
     CborError err = get_string_chunk_size(it, &offset, len);
-    if (err == CborErrorLastStringChunk) {
+    if (err)
-        /* last chunk */
-        if (!cbor_value_is_length_known(it)) {
-            /* skip the break byte */
-            advance_bytes(it, 1);
-        }
-        *bufferptr = NULL;
-        *len = 0;
-        return preparse_next_value(it);
-    } else if (err) {
         return err;
-    }
 
     /* we're good, transfer the string now */
     err = transfer_string(it, bufferptr, offset, *len);
     if (err)
         return err;
 
-    it->flags |= CborIteratorFlag_IteratingStringChunks;
+    /* we've iterated at least once */
+    it->flags &= ~CborIteratorFlag_BeforeFirstStringChunk;
     return CborNoError;
 }
 
-CborError cbor_value_get_string_chunk_size(CborValue *value, size_t *len)
-{
-    size_t offset;
-    return get_string_chunk_size(value, &offset, len);
-}
-
 /**
  * \fn CborError cbor_value_get_text_string_chunk(const CborValue *value, const char **bufferptr, size_t *len, CborValue *next)
  *
@@ -1216,14 +1189,18 @@ static CborError iterate_string_chunks(const CborValue *value, char *buffer, siz
     *next = *value;
     *result = true;
 
+    err = _cbor_value_begin_string_iteration(next);
+    if (err)
+        return err;
+
     while (1) {
         size_t newTotal;
         size_t chunkLen;
         err = get_string_chunk(next, &ptr, &chunkLen);
+        if (err == CborErrorNoMoreStringChunks)
+            break;
         if (err)
             return err;
-        if (!ptr)
-            break;
 
         if (unlikely(add_check_overflow(total, chunkLen, &newTotal)))
             return CborErrorDataTooLarge;
@@ -1242,7 +1219,7 @@ static CborError iterate_string_chunks(const CborValue *value, char *buffer, siz
         *result = !!func(buffer + total, nul, 1);
     }
     *buflen = total;
-    return CborNoError;
+    return _cbor_value_finish_string_iteration(next);
 }
 
 /**

src/3rdparty/tinycbor/tests/parser/tst_parser.cpp

@@ -691,18 +691,23 @@ static void chunkedStringTest(const QByteArray &data, const QString &concatenate
 
     CborValue copy = value;
 
+    err = cbor_value_begin_string_iteration(&value);
+    QVERIFY2(!err, QByteArray("Got error \"") + cbor_error_string(err) + "\"");
     forever {
         QString decoded;
         err = parseOneChunk(&value, &decoded);
-        QVERIFY2(!err, QByteArray("Got error \"") + cbor_error_string(err) + "\"");
+        if (err == CborErrorNoMoreStringChunks)
-
-        if (decoded.isEmpty())
             break;          // last chunk
 
+        QVERIFY2(!err, QByteArray("Got error \"") + cbor_error_string(err) + "\"");
+
         QVERIFY2(!chunks.isEmpty(), "Too many chunks");
         QString expected = chunks.takeFirst();
         QCOMPARE(decoded, expected);
     }
+
+    err = cbor_value_finish_string_iteration(&value);
+    QVERIFY2(!err, QByteArray("Got error \"") + cbor_error_string(err) + "\"");
     QVERIFY2(chunks.isEmpty(), "Too few chunks");
 
     // compare to the concatenated data

src/corelib/serialization/qcborstream.cpp

@@ -1847,11 +1847,6 @@ public:
         IdealIoBufferSize = 256
     };
 
-    struct ChunkParameters {
-        qsizetype offset;
-        qsizetype size;
-    };
-
     QIODevice *device;
     QByteArray buffer;
     QStack<CborValue> containerStack;
@@ -1941,12 +1936,12 @@ public:
         lastError = { QCborError::Code(err) };
     }
 
-    void updateBufferAfterString(ChunkParameters params)
+    void updateBufferAfterString(qsizetype offset, qsizetype size)
     {
         Q_ASSERT(device);
 
-        bufferStart += params.offset;
+        bufferStart += offset;
-        qsizetype newStart = bufferStart + params.size;
+        qsizetype newStart = bufferStart + size;
         qsizetype remainingInBuffer = buffer.size() - newStart;
 
         if (remainingInBuffer <= 0) {
@@ -1962,7 +1957,7 @@ public:
         bufferStart = 0;
     }
 
-    ChunkParameters getStringChunkParameters();
+    bool ensureStringIteration();
 };
 
 void qt_cbor_stream_set_error(QCborStreamReaderPrivate *d, QCborError error)
@@ -2022,30 +2017,16 @@ static CborError qt_cbor_decoder_transfer_string(void *token, const void **userp
     return total > avail ? CborErrorUnexpectedEOF : CborNoError;
 }
 
-QCborStreamReaderPrivate::ChunkParameters QCborStreamReaderPrivate::getStringChunkParameters()
+bool QCborStreamReaderPrivate::ensureStringIteration()
 {
-    size_t len;
+    if (currentElement.flags & CborIteratorFlag_IteratingStringChunks)
-    const void *content = &len;     // set to any non-null value
+        return true;
-    CborError err;
 
-#if 1
+    CborError err = cbor_value_begin_string_iteration(&currentElement);
-    // Using internal TinyCBOR API!
+    if (!err)
-    err = _cbor_value_get_string_chunk(&currentElement, &content, &len, &currentElement);
+        return true;
-#else
-    // the above is effectively the same as:
-    if (cbor_value_is_byte_string(&currentElement))
-        err = cbor_value_get_byte_string_chunk(&currentElement, reinterpret_cast<const uint8_t **>(&content),
-                                               &len, &currentElement);
-    else
-        err = cbor_value_get_text_string_chunk(&currentElement, reinterpret_cast<const char **>(&content),
-                                               &len, &currentElement);
-#endif
-
-    if (err)
     handleError(err);
-    else
+    return false;
-        lastError = {};
-    return { qintptr(content), err ? -1 : qsizetype(len) };
 }
 
 /*!
@@ -2731,9 +2712,12 @@ QCborStreamReader::StringResult<QByteArray> QCborStreamReader::_readByteArray_he
  */
 qsizetype QCborStreamReader::_currentStringChunkSize() const
 {
+    if (!d->ensureStringIteration())
+        return -1;
+
     size_t len;
     CborError err = cbor_value_get_string_chunk_size(&d->currentElement, &len);
-    if (err == CborErrorLastStringChunk)
+    if (err == CborErrorNoMoreStringChunks)
         return 0;           // not a real error
     else if (err)
         d->handleError(err);
@@ -2783,28 +2767,63 @@ qsizetype QCborStreamReader::_currentStringChunkSize() const
 QCborStreamReader::StringResult<qsizetype>
 QCborStreamReader::readStringChunk(char *ptr, qsizetype maxlen)
 {
-    auto params = d->getStringChunkParameters();
+    CborError err;
+    size_t len;
+    const void *content;
     QCborStreamReader::StringResult<qsizetype> result;
     result.data = 0;
     result.status = Error;
 
-    if (params.offset == 0) {
+    d->lastError = {};
-        d->preread();
+    if (!d->ensureStringIteration())
-        preparse();
-        result.status = EndOfString;
-        return result;
-    }
-    if (params.size < 0)
         return result;
 
+#if 1
+    // Using internal TinyCBOR API!
+    err = _cbor_value_get_string_chunk(&d->currentElement, &content, &len, &d->currentElement);
+#else
+    // the above is effectively the same as:
+    if (cbor_value_is_byte_string(&currentElement))
+        err = cbor_value_get_byte_string_chunk(&d->currentElement, reinterpret_cast<const uint8_t **>(&content),
+                                               &len, &d->currentElement);
+    else
+        err = cbor_value_get_text_string_chunk(&d->currentElement, reinterpret_cast<const char **>(&content),
+                                               &len, &d->currentElement);
+#endif
+
+    // Range check: using implementation-defined behavior in converting an
+    // unsigned value out of range of the destination signed type (same as
+    // "len > size_t(std::numeric_limits<qsizetype>::max())", but generates
+    // better code with ICC and MSVC).
+    if (!err && qsizetype(len) < 0)
+        err = CborErrorDataTooLarge;
+
+    if (err) {
+        if (err == CborErrorNoMoreStringChunks) {
+            d->preread();
+            err = cbor_value_finish_string_iteration(&d->currentElement);
+            result.status = EndOfString;
+        }
+        if (err)
+            d->handleError(err);
+        else
+            preparse();
+        return result;
+    }
+
     // Read the chunk into the user's buffer.
-    qsizetype toRead = qMin(maxlen, params.size);
-    qsizetype left = params.size - maxlen;
     qint64 actuallyRead;
+    qptrdiff offset = qptrdiff(content);
+    qsizetype toRead = qsizetype(len);
+    qsizetype left = toRead - maxlen;
+    if (left < 0)
+        left = 0;               // buffer bigger than string
+    else
+        toRead = maxlen;        // buffer smaller than string
 
     if (d->device) {
         // This first skip can't fail because we've already read this many bytes.
-        d->device->skip(d->bufferStart + params.offset);
+        d->device->skip(d->bufferStart + qptrdiff(content));
         actuallyRead = d->device->read(ptr, toRead);
 
         if (actuallyRead != toRead)  {
@@ -2820,11 +2839,11 @@ QCborStreamReader::readStringChunk(char *ptr, qsizetype maxlen)
             return result;
         }
 
-        d->updateBufferAfterString(params);
+        d->updateBufferAfterString(offset, len);
     } else {
         actuallyRead = toRead;
-        memcpy(ptr, d->buffer.constData() + d->bufferStart + params.offset, toRead);
+        memcpy(ptr, d->buffer.constData() + d->bufferStart + offset, toRead);
-        d->bufferStart += QByteArray::size_type(params.offset + params.size);
+        d->bufferStart += QByteArray::size_type(offset + len);
     }
 
     d->preread();