Add [[nodiscard]]-like macro and fix a few violations (#248)

* Add basic test for cbor_builder_string_callback

* Fix string append memory leak and add tests

* Update changelog

* Add unused value attribute to most methods returning values

* Fix some unchecked usages in tests

* Fix bad merge

* Add a changelog entry

CHANGELOG.md

@@ -12,6 +12,8 @@ Next
 - Fix a potential memory leak when the allocator fails during array or map decoding [[#224]](https://github.com/PJK/libcbor/pull/224) (by [James-ZHANG](https://github.com/James-ZHANG))
 - [Fix a memory leak when the allocator fails when adding chunks to indefinite bytestrings.](https://github.com/PJK/libcbor/pull/242) ([discovered](https://github.com/PJK/libcbor/pull/228) by [James-ZHANG](https://github.com/James-ZHANG))
 - [Fix a memory leak when the allocator fails when adding chunks to indefinite strings](https://github.com/PJK/libcbor/pull/246)
+- Potentially BUILD BREAKING: [Add nodiscard attributes to most functions](https://github.com/PJK/libcbor/pull/248)
+  - **Warning**: This may cause new build warnings and (in rare cases, depending on your configuration) errors
 
 
 0.9.0 (2021-11-14)

src/cbor.c

@@ -152,6 +152,9 @@ static cbor_item_t *_cbor_copy_float_ctrl(cbor_item_t *item) {
 }
 
 cbor_item_t *cbor_copy(cbor_item_t *item) {
+  // TODO: Handle memory allocation error -- currently the code will produce
+  // partial or invalid items on failure
+
   // cppcheck-suppress missingReturn
   switch (cbor_typeof(item)) {
     case CBOR_TYPE_UINT:

src/cbor.h

@@ -43,8 +43,8 @@ extern "C" {
  * @return **new** CBOR item or `NULL` on failure. In that case, \p result
  * contains location and description of the error.
  */
-CBOR_EXPORT cbor_item_t* cbor_load(cbor_data source, size_t source_size,
-                                   struct cbor_load_result* result);
+_CBOR_NODISCARD CBOR_EXPORT cbor_item_t* cbor_load(
+    cbor_data source, size_t source_size, struct cbor_load_result* result);
 
 /** Deep copy of an item
  *
@@ -53,7 +53,7 @@ CBOR_EXPORT cbor_item_t* cbor_load(cbor_data source, size_t source_size,
  * @param item[borrow] item to copy
  * @return **new** CBOR deep copy
  */
-CBOR_EXPORT cbor_item_t* cbor_copy(cbor_item_t* item);
+_CBOR_NODISCARD CBOR_EXPORT cbor_item_t* cbor_copy(cbor_item_t* item);
 
 #if CBOR_PRETTY_PRINTER
 #include <stdio.h>

src/cbor/arrays.h

@@ -20,13 +20,15 @@ extern "C" {
  * @param item[borrow] An array
  * @return The number of members
  */
+_CBOR_NODISCARD
 CBOR_EXPORT size_t cbor_array_size(const cbor_item_t* item);
 
 /** Get the size of the allocated storage
  *
  * @param item[borrow] An array
  * @return The size of the allocated storage (number of items)
  */
+_CBOR_NODISCARD
 CBOR_EXPORT size_t cbor_array_allocated(const cbor_item_t* item);
 
 /** Get item by index
@@ -35,6 +37,7 @@ CBOR_EXPORT size_t cbor_array_allocated(const cbor_item_t* item);
  * @param index The index
  * @return **incref** The item, or `NULL` in case of boundary violation
  */
+_CBOR_NODISCARD
 CBOR_EXPORT cbor_item_t* cbor_array_get(const cbor_item_t* item, size_t index);
 
 /** Set item by index
@@ -47,6 +50,7 @@ CBOR_EXPORT cbor_item_t* cbor_array_get(const cbor_item_t* item, size_t index);
  * @param index The index, first item is 0.
  * @return true on success, false on allocation failure.
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_array_set(cbor_item_t* item, size_t index,
                                 cbor_item_t* value);
 
@@ -59,6 +63,7 @@ CBOR_EXPORT bool cbor_array_set(cbor_item_t* item, size_t index,
  * @param index The index, first item is 0.
  * @return true on success, false on allocation failure.
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_array_replace(cbor_item_t* item, size_t index,
                                     cbor_item_t* value);
 
@@ -67,13 +72,15 @@ CBOR_EXPORT bool cbor_array_replace(cbor_item_t* item, size_t index,
  * @param item[borrow] An array
  * @return Is the array definite?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_array_is_definite(const cbor_item_t* item);
 
 /** Is the array indefinite?
  *
  * @param item[borrow] An array
  * @return Is the array indefinite?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_array_is_indefinite(const cbor_item_t* item);
 
 /** Get the array contents
@@ -84,19 +91,22 @@ CBOR_EXPORT bool cbor_array_is_indefinite(const cbor_item_t* item);
  * @param item[borrow] An array
  * @return #cbor_array_size items
  */
+_CBOR_NODISCARD
 CBOR_EXPORT cbor_item_t** cbor_array_handle(const cbor_item_t* item);
 
 /** Create new definite array
  *
  * @param size Number of slots to preallocate
  * @return **new** array or `NULL` upon malloc failure
  */
+_CBOR_NODISCARD
 CBOR_EXPORT cbor_item_t* cbor_new_definite_array(size_t size);
 
 /** Create new indefinite array
  *
  * @return **new** array or `NULL` upon malloc failure
  */
+_CBOR_NODISCARD
 CBOR_EXPORT cbor_item_t* cbor_new_indefinite_array(void);
 
 /** Append to the end
@@ -108,6 +118,7 @@ CBOR_EXPORT cbor_item_t* cbor_new_indefinite_array(void);
  * @param pushee[incref] The item to push
  * @return true on success, false on failure
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_array_push(cbor_item_t* array, cbor_item_t* pushee);
 
 #ifdef __cplusplus

src/cbor/bytestrings.h

@@ -28,20 +28,23 @@ extern "C" {
  * @param item[borrow] a definite bytestring
  * @return length of the binary data. Zero if no chunk has been attached yet
  */
+_CBOR_NODISCARD
 CBOR_EXPORT size_t cbor_bytestring_length(const cbor_item_t *item);
 
 /** Is the byte string definite?
  *
  * @param item[borrow] a byte string
  * @return Is the byte string definite?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_bytestring_is_definite(const cbor_item_t *item);
 
 /** Is the byte string indefinite?
  *
  * @param item[borrow] a byte string
  * @return Is the byte string indefinite?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_bytestring_is_indefinite(const cbor_item_t *item);
 
 /** Get the handle to the binary data
@@ -53,6 +56,7 @@ CBOR_EXPORT bool cbor_bytestring_is_indefinite(const cbor_item_t *item);
  * @return The address of the binary data. `NULL` if no data have been assigned
  * yet.
  */
+_CBOR_NODISCARD
 CBOR_EXPORT cbor_mutable_data cbor_bytestring_handle(const cbor_item_t *item);
 
 /** Set the handle to the binary data
@@ -74,6 +78,7 @@ CBOR_EXPORT void cbor_bytestring_set_handle(
  * @param item[borrow] A indefinite byte string
  * @return array of #cbor_bytestring_chunk_count definite bytestrings
  */
+_CBOR_NODISCARD
 CBOR_EXPORT cbor_item_t **cbor_bytestring_chunks_handle(
     const cbor_item_t *item);
 
@@ -82,6 +87,7 @@ CBOR_EXPORT cbor_item_t **cbor_bytestring_chunks_handle(
  * @param item[borrow] A indefinite bytestring
  * @return The chunk count. 0 for freshly created items.
  */
+_CBOR_NODISCARD
 CBOR_EXPORT size_t cbor_bytestring_chunk_count(const cbor_item_t *item);
 
 /** Appends a chunk to the bytestring
@@ -95,6 +101,7 @@ CBOR_EXPORT size_t cbor_bytestring_chunk_count(const cbor_item_t *item);
  * @return true on success, false on realloc failure. In that case, the refcount
  * of `chunk` is not increased and the `item` is left intact.
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_bytestring_add_chunk(cbor_item_t *item,
                                            cbor_item_t *chunk);
 
@@ -104,6 +111,7 @@ CBOR_EXPORT bool cbor_bytestring_add_chunk(cbor_item_t *item,
  *
  * @return **new** definite bytestring. `NULL` on malloc failure.
  */
+_CBOR_NODISCARD
 CBOR_EXPORT cbor_item_t *cbor_new_definite_bytestring(void);
 
 /** Creates a new indefinite byte string
@@ -112,6 +120,7 @@ CBOR_EXPORT cbor_item_t *cbor_new_definite_bytestring(void);
  *
  * @return **new** indefinite bytestring. `NULL` on malloc failure.
  */
+_CBOR_NODISCARD
 CBOR_EXPORT cbor_item_t *cbor_new_indefinite_bytestring(void);
 
 /** Creates a new byte string and initializes it
@@ -123,6 +132,7 @@ CBOR_EXPORT cbor_item_t *cbor_new_indefinite_bytestring(void);
  * @return A **new** byte string with content `handle`. `NULL` on malloc
  * failure.
  */
+_CBOR_NODISCARD
 CBOR_EXPORT cbor_item_t *cbor_build_bytestring(cbor_data handle, size_t length);
 
 #ifdef __cplusplus

src/cbor/common.h

@@ -67,10 +67,15 @@ static const uint8_t cbor_patch_version = CBOR_PATCH_VERSION;
 
 #ifdef __GNUC__
 #define _CBOR_UNUSED(x) __attribute__((__unused__)) x
+// TODO(https://github.com/PJK/libcbor/issues/247): Prefer [[nodiscard]] if
+// available
+#define _CBOR_NODISCARD __attribute__((warn_unused_result))
 #elif defined(_MSC_VER)
 #define _CBOR_UNUSED(x) __pragma(warning(suppress : 4100 4101)) x
+#define _CBOR_NODISCARD
 #else
 #define _CBOR_UNUSED(x) x
+#define _CBOR_NODISCARD
 #endif
 
 // Macro to short-circuit builder functions when memory allocation fails
@@ -146,6 +151,7 @@ CBOR_EXPORT void cbor_set_allocs(_cbor_malloc_t custom_malloc,
  * @param item[borrow]
  * @return The type
  */
+_CBOR_NODISCARD
 CBOR_EXPORT cbor_type cbor_typeof(
     const cbor_item_t *item); /* Will be inlined iff link-time opt is enabled */
 
@@ -155,48 +161,56 @@ CBOR_EXPORT cbor_type cbor_typeof(
  * @param item[borrow] the item
  * @return Is the item an #CBOR_TYPE_UINT?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_isa_uint(const cbor_item_t *item);
 
 /** Does the item have the appropriate major type?
  * @param item[borrow] the item
  * @return Is the item a #CBOR_TYPE_NEGINT?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_isa_negint(const cbor_item_t *item);
 
 /** Does the item have the appropriate major type?
  * @param item[borrow] the item
  * @return Is the item a #CBOR_TYPE_BYTESTRING?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_isa_bytestring(const cbor_item_t *item);
 
 /** Does the item have the appropriate major type?
  * @param item[borrow] the item
  * @return Is the item a #CBOR_TYPE_STRING?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_isa_string(const cbor_item_t *item);
 
 /** Does the item have the appropriate major type?
  * @param item[borrow] the item
  * @return Is the item an #CBOR_TYPE_ARRAY?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_isa_array(const cbor_item_t *item);
 
 /** Does the item have the appropriate major type?
  * @param item[borrow] the item
  * @return Is the item a #CBOR_TYPE_MAP?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_isa_map(const cbor_item_t *item);
 
 /** Does the item have the appropriate major type?
  * @param item[borrow] the item
  * @return Is the item a #CBOR_TYPE_TAG?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_isa_tag(const cbor_item_t *item);
 
 /** Does the item have the appropriate major type?
  * @param item[borrow] the item
  * @return Is the item a #CBOR_TYPE_FLOAT_CTRL?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_isa_float_ctrl(const cbor_item_t *item);
 
 /* Practical types with respect to their semantics (but not tag values) */
@@ -205,18 +219,21 @@ CBOR_EXPORT bool cbor_isa_float_ctrl(const cbor_item_t *item);
  * @param item[borrow] the item
  * @return  Is the item an integer, either positive or negative?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_is_int(const cbor_item_t *item);
 
 /** Is the item an a floating point number?
  * @param item[borrow] the item
  * @return  Is the item a floating point number?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_is_float(const cbor_item_t *item);
 
 /** Is the item an a boolean?
  * @param item[borrow] the item
  * @return  Is the item a boolean?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_is_bool(const cbor_item_t *item);
 
 /** Does this item represent `null`
@@ -229,6 +246,7 @@ CBOR_EXPORT bool cbor_is_bool(const cbor_item_t *item);
  * @param item[borrow] the item
  * @return  Is the item (CBOR logical) null?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_is_null(const cbor_item_t *item);
 
 /** Does this item represent `undefined`
@@ -241,6 +259,7 @@ CBOR_EXPORT bool cbor_is_null(const cbor_item_t *item);
  * @param item[borrow] the item
  * @return Is the item (CBOR logical) undefined?
  */
+_CBOR_NODISCARD
 CBOR_EXPORT bool cbor_is_undef(const cbor_item_t *item);
 
 /*
@@ -285,6 +304,7 @@ CBOR_EXPORT void cbor_intermediate_decref(cbor_item_t *item);
  * @param item[borrow] the item
  * @return the reference count
  */
+_CBOR_NODISCARD
 CBOR_EXPORT size_t cbor_refcount(const cbor_item_t *item);
 
 /** Provides CPP-like move construct
@@ -302,6 +322,7 @@ CBOR_EXPORT size_t cbor_refcount(const cbor_item_t *item);
  * @param item[take] the item
  * @return the item with reference count decreased by one
  */
+_CBOR_NODISCARD
 CBOR_EXPORT cbor_item_t *cbor_move(cbor_item_t *item);
 
 #ifdef __cplusplus