Merge pull request #193 from PJK/pk/touchups

Typo fixes and TODOs cleanup

.gitignore

@@ -8,6 +8,7 @@ cmake-build-debug
 venv
 **.DS_Store
 .vscode
+# No top-level requirements, see doc/source
 requirements.txt
 examples/bazel/bazel-bazel
 examples/bazel/bazel-bin

src/cbor.c

@@ -83,7 +83,7 @@ cbor_item_t *cbor_load(cbor_data source, size_t source_size,
           goto error;
         }
       case CBOR_DECODER_ERROR:
-        /* Reserved/malformated item */
+        /* Reserved/malformed item */
         {
           result->error.code = CBOR_ERR_MALFORMATED;
           goto error;
@@ -245,13 +245,13 @@ static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) {
       fprintf(out, "Width: %dB, ", _pow(2, cbor_int_get_width(item)));
       fprintf(out, "Value: %" PRIu64 "\n", cbor_get_int(item));
       break;
-    };
+    }
     case CBOR_TYPE_NEGINT: {
       fprintf(out, "%*s[CBOR_TYPE_NEGINT] ", indent, " ");
       fprintf(out, "Width: %dB, ", _pow(2, cbor_int_get_width(item)));
       fprintf(out, "Value: -%" PRIu64 " -1\n", cbor_get_int(item));
       break;
-    };
+    }
     case CBOR_TYPE_BYTESTRING: {
       fprintf(out, "%*s[CBOR_TYPE_BYTESTRING] ", indent, " ");
       if (cbor_bytestring_is_indefinite(item)) {
@@ -264,7 +264,7 @@ static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) {
         fprintf(out, "Definite, length %zuB\n", cbor_bytestring_length(item));
       }
       break;
-    };
+    }
     case CBOR_TYPE_STRING: {
       fprintf(out, "%*s[CBOR_TYPE_STRING] ", indent, " ");
       if (cbor_string_is_indefinite(item)) {
@@ -285,7 +285,7 @@ static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) {
         fprintf(out, "\n");
       }
       break;
-    };
+    }
     case CBOR_TYPE_ARRAY: {
       fprintf(out, "%*s[CBOR_TYPE_ARRAY] ", indent, " ");
       if (cbor_array_is_definite(item)) {
@@ -297,7 +297,7 @@ static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) {
       for (size_t i = 0; i < cbor_array_size(item); i++)
         _cbor_nested_describe(cbor_array_handle(item)[i], out, indent + 4);
       break;
-    };
+    }
     case CBOR_TYPE_MAP: {
       fprintf(out, "%*s[CBOR_TYPE_MAP] ", indent, " ");
       if (cbor_map_is_definite(item)) {
@@ -311,13 +311,13 @@ static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) {
         _cbor_nested_describe(cbor_map_handle(item)[i].value, out, indent + 4);
       }
       break;
-    };
+    }
     case CBOR_TYPE_TAG: {
       fprintf(out, "%*s[CBOR_TYPE_TAG] ", indent, " ");
       fprintf(out, "Value: %" PRIu64 "\n", cbor_tag_value(item));
       _cbor_nested_describe(cbor_move(cbor_tag_item(item)), out, indent + 4);
       break;
-    };
+    }
     case CBOR_TYPE_FLOAT_CTRL: {
       fprintf(out, "%*s[CBOR_TYPE_FLOAT_CTRL] ", indent, " ");
       if (cbor_float_ctrl_is_ctrl(item)) {
@@ -334,7 +334,7 @@ static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) {
         fprintf(out, "value: %lf\n", cbor_float_get_float(item));
       }
       break;
-    };
+    }
   }
 }
 

src/cbor/arrays.c

@@ -31,9 +31,6 @@ bool cbor_array_set(cbor_item_t *item, size_t index, cbor_item_t *value) {
   } else {
     return false;
   }
-  // TODO: This is unreachable and the index checking logic above seems
-  // suspicious -- out of bounds index is a caller error. Figure out & fix.
-  return true;
 }
 
 bool cbor_array_replace(cbor_item_t *item, size_t index, cbor_item_t *value) {

src/cbor/arrays.h

@@ -39,7 +39,8 @@ CBOR_EXPORT cbor_item_t* cbor_array_get(const cbor_item_t* item, size_t index);
 
 /** Set item by index
  *
- * Creating arrays with holes is not possible
+ * If the index is out of bounds, the array is not modified and false is
+ * returned. Creating arrays with holes is not possible.
  *
  * @param item[borrow] An array
  * @param value[incref] The item to assign
@@ -100,7 +101,7 @@ CBOR_EXPORT cbor_item_t* cbor_new_indefinite_array();
 
 /** Append to the end
  *
- * For indefinite items, storage may be realloacted. For definite items, only
+ * For indefinite items, storage may be reallocated. For definite items, only
  * the preallocated capacity is available.
  *
  * @param array[borrow] An array

src/cbor/bytestrings.c

@@ -34,7 +34,8 @@ cbor_item_t *cbor_new_definite_bytestring() {
   *item = (cbor_item_t){
       .refcount = 1,
       .type = CBOR_TYPE_BYTESTRING,
-      .metadata = {.bytestring_metadata = {_CBOR_METADATA_DEFINITE, 0}}};
+      .metadata = {.bytestring_metadata = {.type = _CBOR_METADATA_DEFINITE,
+                                           .length = 0}}};
   return item;
 }
 

src/cbor/bytestrings.h

@@ -108,7 +108,7 @@ CBOR_EXPORT cbor_item_t *cbor_new_definite_bytestring();
 
 /** Creates a new indefinite byte string
  *
- * The chunks array is initialized to `NULL` and chunkcount to 0
+ * The chunks array is initialized to `NULL` and chunk count to 0
  *
  * @return **new** indefinite bytestring. `NULL` on malloc failure.
  */

src/cbor/common.c

@@ -132,7 +132,7 @@ void cbor_decref(cbor_item_t **item_ref) {
         }
         _CBOR_FREE(item->data);
         break;
-      };
+      }
       case CBOR_TYPE_TAG: {
         if (item->metadata.tag_metadata.tagged_item != NULL)
           cbor_decref(&item->metadata.tag_metadata.tagged_item);
@@ -145,7 +145,6 @@ void cbor_decref(cbor_item_t **item_ref) {
       }
     }
     _CBOR_FREE(item);
-    // TODO
     *item_ref = NULL;
   }
 }

src/cbor/common.h

@@ -21,7 +21,7 @@
 extern "C" {
 
 /**
- * C++ is not a subset of C99 -- 'restrict' qualifier is not a part of the
+ * C99 is not a subset of C++ -- 'restrict' qualifier is not a part of the
  * language. This is a workaround to keep it in C headers -- compilers allow
  * linking non-restrict signatures with restrict implementations.
  *

src/cbor/data.h

@@ -45,6 +45,8 @@ typedef enum {
   CBOR_ERR_NONE,
   CBOR_ERR_NOTENOUGHDATA,
   CBOR_ERR_NODATA,
+  // TODO: Should be "malformed" or at least "malformatted". Retained for
+  // backwards compatibility.
   CBOR_ERR_MALFORMATED,
   CBOR_ERR_MEMERROR /** Memory error - item allocation failed. Is it too big for
                        your allocator? */
@@ -217,6 +219,8 @@ enum cbor_decoder_status {
    */
   CBOR_DECODER_FINISHED,
   /** Not enough data to invoke a callback */
+  // TODO: The name is inconsistent with CBOR_ERR_NOTENOUGHDATA. Retained for
+  // backwards compatibility.
   CBOR_DECODER_NEDATA,
   /** Bad data (reserved MTB, malformed value, etc.)  */
   CBOR_DECODER_ERROR

src/cbor/floats_ctrls.h

@@ -48,7 +48,7 @@ CBOR_EXPORT float cbor_float_get_float2(const cbor_item_t *item);
  *
  * The item must have the corresponding width
  *
- * @param[borrow] A signle precision float
+ * @param[borrow] A single precision float
  * @return single precision value
  */
 CBOR_EXPORT float cbor_float_get_float4(const cbor_item_t *item);
@@ -125,7 +125,7 @@ CBOR_EXPORT cbor_item_t *cbor_new_undef();
 /** Constructs new boolean ctrl item
  *
  * @param value The value to use
- * @return **new** boolen ctrl item or `NULL` upon memory allocation failure
+ * @return **new** boolean ctrl item or `NULL` upon memory allocation failure
  */
 CBOR_EXPORT cbor_item_t *cbor_build_bool(bool value);
 

src/cbor/internal/builder_callbacks.c

@@ -233,7 +233,7 @@ void cbor_builder_string_callback(void *context, cbor_data data,
   uint64_t codepoint_count =
       _cbor_unicode_codepoint_count(data, length, &unicode_status);
 
-  if (unicode_status.status == _CBOR_UNICODE_BADCP) {
+  if (unicode_status.status != _CBOR_UNICODE_OK) {
     ctx->syntax_error = true;
     return;
   }

src/cbor/internal/unicode.c

@@ -6,6 +6,7 @@
  */
 
 #include "unicode.h"
+#include <stdint.h>
 
 #define UTF8_ACCEPT 0
 #define UTF8_REJECT 1
@@ -90,5 +91,5 @@ uint64_t _cbor_unicode_codepoint_count(cbor_data source, uint64_t source_length,
 error:
   *status = (struct _cbor_unicode_status){.location = pos,
                                           .status = _CBOR_UNICODE_BADCP};
-  return -1;
+  return 0;
 }

src/cbor/maps.h

@@ -57,7 +57,7 @@ CBOR_EXPORT cbor_item_t *cbor_new_indefinite_map();
  * @param item[borrow] A map
  * @param pair[incref] The key-value pair to add (incref is member-wise)
  * @return `true` on success, `false` if either reallocation failed or the
- * preallcoated storage is full
+ * preallocated storage is full
  */
 CBOR_EXPORT bool cbor_map_add(cbor_item_t *item, struct cbor_pair pair);
 
@@ -68,7 +68,7 @@ CBOR_EXPORT bool cbor_map_add(cbor_item_t *item, struct cbor_pair pair);
  * @param item[borrow] A map
  * @param key[incref] The key
  * @return `true` on success, `false` if either reallocation failed or the
- * preallcoated storage is full
+ * preallocated storage is full
  */
 CBOR_EXPORT bool _cbor_map_add_key(cbor_item_t *item, cbor_item_t *key);
 
@@ -79,7 +79,7 @@ CBOR_EXPORT bool _cbor_map_add_key(cbor_item_t *item, cbor_item_t *key);
  * @param item[borrow] A map
  * @param key[incref] The value
  * @return `true` on success, `false` if either reallocation failed or the
- * preallcoated storage is full
+ * preallocated storage is full
  */
 CBOR_EXPORT bool _cbor_map_add_value(cbor_item_t *item, cbor_item_t *value);
 

src/cbor/serialization.c

@@ -51,7 +51,7 @@ size_t cbor_serialize_alloc(const cbor_item_t *item, unsigned char **buffer,
 
   size_t written;
 
-  /* This is waaay too optimistic - figure out something smarter (eventually) */
+  // TODO: Would it be possible to tell the size upfront?
   while ((written = cbor_serialize(item, bfr, bfr_size)) == 0) {
     if (!_cbor_safe_to_multiply(CBOR_BUFFER_GROWTH, bfr_size)) {
       _CBOR_FREE(bfr);

src/cbor/serialization.h

@@ -60,7 +60,7 @@ CBOR_EXPORT size_t cbor_serialize_uint(const cbor_item_t *, cbor_mutable_data,
 
 /** Serialize a negint
  *
- * @param item[borrow] A neging
+ * @param item[borrow] A negint
  * @param buffer Buffer to serialize to
  * @param buffer_size Size of the \p buffer
  * @return Length of the result. 0 on failure.

src/cbor/streaming.c

@@ -450,8 +450,8 @@ struct cbor_decoder_result cbor_stream_decode(
     case 0xC5:
       /* Big float */
       {
-        callbacks->tag(context,
-                       _cbor_load_uint8(source) - 0xC0); /* 0xC0 offset */
+        callbacks->tag(context, (uint64_t)(_cbor_load_uint8(source) -
+                                           0xC0)); /* 0xC0 offset */
         return result;
       }
     case 0xC6: /* Fallthrough */
@@ -476,8 +476,8 @@ struct cbor_decoder_result cbor_stream_decode(
     case 0xD6: /* Expected b64 conversion tag - fallthrough */
     case 0xD7: /* Expected b16 conversion tag */
     {
-      callbacks->tag(context,
-                     _cbor_load_uint8(source) - 0xC0); /* 0xC0 offset */
+      callbacks->tag(context, (uint64_t)(_cbor_load_uint8(source) -
+                                         0xC0)); /* 0xC0 offset */
       return result;
     }
     case 0xD8: /* 1B tag */

test/unicode_test.c

@@ -18,19 +18,25 @@ unsigned char missing_bytes_data[] = {0xC4, 0x8C};
 
 /* Capital accented C */
 static void test_missing_bytes(void **_CBOR_UNUSED(_state)) {
-  _cbor_unicode_codepoint_count(missing_bytes_data, 1, &status);
+  assert_int_equal(
+      _cbor_unicode_codepoint_count(missing_bytes_data, 1, &status), 0);
   assert_true(status.status == _CBOR_UNICODE_BADCP);
-  _cbor_unicode_codepoint_count(missing_bytes_data, 2, &status);
+  assert_int_equal(status.location, 1);
+
+  assert_int_equal(
+      _cbor_unicode_codepoint_count(missing_bytes_data, 2, &status), 1);
   assert_true(status.status == _CBOR_UNICODE_OK);
+  assert_int_equal(status.location, 0);
 }
 
 unsigned char invalid_sequence_data[] = {0x65, 0xC4, 0x00};
 
 /* e, invalid seq */
 static void test_invalid_sequence(void **_CBOR_UNUSED(_state)) {
-  _cbor_unicode_codepoint_count(invalid_sequence_data, 3, &status);
+  assert_int_equal(
+      _cbor_unicode_codepoint_count(invalid_sequence_data, 3, &status), 0);
   assert_true(status.status == _CBOR_UNICODE_BADCP);
-  assert_true(status.location == 2);
+  assert_int_equal(status.location, 2);
 }
 
 int main(void) {