Encoder: Change the way we reckon the number of items added

Instead of counting forward the number of items added, which meant we
couldn't know in cbot_encoder_close_container() whether we had added
enough, let's count backwards. The number is offset by 1 so we should be
at 1 if we added exactly as many items as we had expected to. Zero means
we added too many. Any other number means we added too few.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>

Author: thiagomacieira

src/cbor.h

@@ -209,7 +209,7 @@ struct CborEncoder
         ptrdiff_t bytes_needed;
     } data;
     const uint8_t *end;
-    size_t added;
+    size_t remaining;
     int flags;
 };
 typedef struct CborEncoder CborEncoder;

src/cborencoder.c

@@ -204,7 +204,7 @@ void cbor_encoder_init(CborEncoder *encoder, uint8_t *buffer, size_t size, int f
 {
     encoder->data.ptr = buffer;
     encoder->end = buffer + size;
-    encoder->added = 0;
+    encoder->remaining = 2;
     encoder->flags = flags;
 }
 
@@ -305,9 +305,15 @@ static inline CborError encode_number_no_update(CborEncoder *encoder, uint64_t u
     return append_to_buffer(encoder, bufstart, bufend - bufstart);
 }
 
+static inline void saturated_decrement(CborEncoder *encoder)
+{
+    if (encoder->remaining)
+        --encoder->remaining;
+}
+
 static inline CborError encode_number(CborEncoder *encoder, uint64_t ui, uint8_t shiftedMajorType)
 {
-    ++encoder->added;
+    saturated_decrement(encoder);
     return encode_number_no_update(encoder, ui, shiftedMajorType);
 }
 
@@ -389,7 +395,7 @@ CborError cbor_encode_floating_point(CborEncoder *encoder, CborType fpType, cons
         put32(buf + 1, *(const uint32_t*)value);
     else
         put16(buf + 1, *(const uint16_t*)value);
-    ++encoder->added;
+    saturated_decrement(encoder);
     return append_to_buffer(encoder, buf, size + 1);
 }
 
@@ -454,8 +460,8 @@ static CborError create_container(CborEncoder *encoder, CborEncoder *container,
     CborError err;
     container->data.ptr = encoder->data.ptr;
     container->end = encoder->end;
-    ++encoder->added;
-    container->added = 0;
+    saturated_decrement(encoder);
+    container->remaining = length + 1;      /* overflow ok on CborIndefiniteLength */
 
     cbor_static_assert(((MapType << MajorTypeShift) & CborIteratorFlag_ContainerIsMap) == CborIteratorFlag_ContainerIsMap);
     cbor_static_assert(((ArrayType << MajorTypeShift) & CborIteratorFlag_ContainerIsMap) == 0);
@@ -465,6 +471,8 @@ static CborError create_container(CborEncoder *encoder, CborEncoder *container,
         container->flags |= CborIteratorFlag_UnknownLength;
         err = append_byte_to_buffer(container, shiftedMajorType + IndefiniteLength);
     } else {
+        if (shiftedMajorType & CborIteratorFlag_ContainerIsMap)
+            container->remaining += length;
         err = encode_number_no_update(container, length, shiftedMajorType);
     }
     return err;
@@ -520,8 +528,8 @@ CborError cbor_encoder_create_map(CborEncoder *encoder, CborEncoder *mapEncoder,
  * same as were passed to cbor_encoder_create_array() or
  * cbor_encoder_create_map().
  *
- * This function does not verify that the number of items (or pair of items, in
- * the case of a map) was correct. To execute that verification, call
+ * Since version 0.5, this function verifies that the number of items (or pair
+ * of items, in the case of a map) was correct. It is no longer needed to call
  * cbor_encoder_close_container_checked() instead.
  *
  * \sa cbor_encoder_create_array(), cbor_encoder_create_map()
@@ -535,6 +543,10 @@ CborError cbor_encoder_close_container(CborEncoder *encoder, const CborEncoder *
     encoder->end = containerEncoder->end;
     if (containerEncoder->flags & CborIteratorFlag_UnknownLength)
         return append_byte_to_buffer(encoder, BreakByte);
+
+    if (containerEncoder->remaining != 1)
+        return containerEncoder->remaining == 0 ? CborErrorTooManyItems : CborErrorTooFewItems;
+
     if (!encoder->end)
         return CborErrorOutOfMemory;    /* keep the state */
     return CborNoError;

src/cborencoder_close_container_checked.c

@@ -29,49 +29,29 @@
 #endif
 
 #include "cbor.h"
-#include "cborinternal_p.h"
-#include "compilersupport_p.h"
 
 /**
  * \addtogroup CborEncoding
  * @{
  */
 
 /**
+ * @deprecated
  *
  * Closes the CBOR container (array or map) provided by \a containerEncoder and
  * updates the CBOR stream provided by \a encoder. Both parameters must be the
  * same as were passed to cbor_encoder_create_array() or
  * cbor_encoder_create_map().
  *
- * Unlike cbor_encoder_close_container(), this function checks that the number
- * of items (or pair of items, in the case of a map) was correct. If the number
- * of items inserted does not match the length originally passed to
- * cbor_encoder_create_array() or cbor_encoder_create_map(), this function
- * returns either CborErrorTooFewItems or CborErrorTooManyItems.
+ * Prior to version 0.5, cbor_encoder_close_container() did not check the
+ * number of items added. Since that version, it does and now
+ * cbor_encoder_close_container_checked() is no longer needed.
  *
  * \sa cbor_encoder_create_array(), cbor_encoder_create_map()
  */
 CborError cbor_encoder_close_container_checked(CborEncoder *encoder, const CborEncoder *containerEncoder)
 {
-    const uint8_t *ptr = encoder->data.ptr;
-    CborError err = cbor_encoder_close_container(encoder, containerEncoder);
-    if (containerEncoder->flags & CborIteratorFlag_UnknownLength || encoder->end == NULL)
-        return err;
-
-    /* check what the original length was */
-    uint64_t actually_added;
-    err = _cbor_value_extract_number(&ptr, encoder->data.ptr, &actually_added);
-    if (err)
-        return err;
-
-    if (containerEncoder->flags & CborIteratorFlag_ContainerIsMap) {
-        if (actually_added > SIZE_MAX / 2)
-            return CborErrorDataTooLarge;
-        actually_added *= 2;
-    }
-    return actually_added == containerEncoder->added ? CborNoError :
-           actually_added < containerEncoder->added ? CborErrorTooManyItems : CborErrorTooFewItems;
+    return cbor_encoder_close_container(encoder, containerEncoder);
 }
 
 /** @} */

tests/encoder/tst_encoder.cpp

@@ -276,7 +276,7 @@ void compare(const QVariant &input, const QByteArray &output)
     cbor_encoder_init(&encoder, bufptr, buffer.length(), 0);
 
     QCOMPARE(encodeVariant(&encoder, input), CborNoError);
-    QCOMPARE(encoder.added, size_t(1));
+    QCOMPARE(encoder.remaining, size_t(1));
     QCOMPARE(cbor_encoder_get_extra_bytes_needed(&encoder), size_t(0));
 
     buffer.resize(int(cbor_encoder_get_buffer_size(&encoder, bufptr)));
@@ -642,7 +642,7 @@ void tst_Encoder::tooShortArrays()
     cbor_encoder_init(&encoder, reinterpret_cast<quint8 *>(buffer.data()), buffer.length(), 0);
     QCOMPARE(cbor_encoder_create_array(&encoder, &container, 2), CborNoError);
     QCOMPARE(encodeVariant(&container, input), CborNoError);
-    QCOMPARE(container.added, size_t(1));
+    QCOMPARE(container.remaining, size_t(2));
     QCOMPARE(cbor_encoder_close_container_checked(&encoder, &container), CborErrorTooFewItems);
 }
 
@@ -656,7 +656,7 @@ void tst_Encoder::tooShortMaps()
     cbor_encoder_init(&encoder, reinterpret_cast<quint8 *>(buffer.data()), buffer.length(), 0);
     QCOMPARE(cbor_encoder_create_map(&encoder, &container, 2), CborNoError);
     QCOMPARE(encodeVariant(&container, input), CborNoError);
-    QCOMPARE(container.added, size_t(1));
+    QCOMPARE(container.remaining, size_t(4));
     QCOMPARE(cbor_encoder_close_container_checked(&encoder, &container), CborErrorTooFewItems);
 }
 
@@ -671,7 +671,7 @@ void tst_Encoder::tooBigArrays()
     QCOMPARE(cbor_encoder_create_array(&encoder, &container, 1), CborNoError);
     QCOMPARE(encodeVariant(&container, input), CborNoError);
     QCOMPARE(encodeVariant(&container, input), CborNoError);
-    QCOMPARE(container.added, size_t(2));
+    QCOMPARE(container.remaining, size_t(0));
     QCOMPARE(cbor_encoder_close_container_checked(&encoder, &container), CborErrorTooManyItems);
 }
 
@@ -687,7 +687,7 @@ void tst_Encoder::tooBigMaps()
     QCOMPARE(encodeVariant(&container, input), CborNoError);
     QCOMPARE(encodeVariant(&container, input), CborNoError);
     QCOMPARE(encodeVariant(&container, input), CborNoError);
-    QCOMPARE(container.added, size_t(3));
+    QCOMPARE(container.remaining, size_t(0));
     QCOMPARE(cbor_encoder_close_container_checked(&encoder, &container), CborErrorTooManyItems);
 }