From 9caf9f31df09daa4ef49f4d48b21f84317877faa Mon Sep 17 00:00:00 2001 From: Dengke Date: Thu, 20 Jun 2024 14:30:47 -0700 Subject: [PATCH] comments addressed --- CMakeLists.txt | 1 - include/aws/crt/cbor/Cbor.h | 65 ++++++++-------- source/cbor/Cbor.cpp | 74 ++++++++++-------- tests/CborTest.cpp | 149 ++++++++++++++++-------------------- 4 files changed, 144 insertions(+), 145 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7623aed10..fb54a14e0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -194,7 +194,6 @@ endif() file(GLOB AWS_CRT_CPP_HEADERS ${AWS_CRT_PUBLIC_HEADERS} - ${AWS_CRT_EXTERNAL_HEADERS} ) file(GLOB AWS_CRT_SRC diff --git a/include/aws/crt/cbor/Cbor.h b/include/aws/crt/cbor/Cbor.h index d082ff978..3cc120cb7 100644 --- a/include/aws/crt/cbor/Cbor.h +++ b/include/aws/crt/cbor/Cbor.h @@ -17,7 +17,7 @@ namespace Aws /** * The types used by APIs, not 1:1 with major types. * It's an extension for CBOR major type in RFC8949 section 3.1. - * Major type 0 - Uint + * Major type 0 - UInt * Major type 1 - NegInt * Major type 2 - Bytes / IndefBytesStart * Major type 3 - Text / IndefTextStart @@ -35,7 +35,7 @@ namespace Aws enum class CborType { Unknown = AWS_CBOR_TYPE_UNKNOWN, - Uint = AWS_CBOR_TYPE_UINT, + UInt = AWS_CBOR_TYPE_UINT, NegInt = AWS_CBOR_TYPE_NEGINT, Float = AWS_CBOR_TYPE_FLOAT, Bytes = AWS_CBOR_TYPE_BYTES, @@ -61,7 +61,7 @@ namespace Aws CborEncoder &operator=(const CborEncoder &) = delete; CborEncoder &operator=(CborEncoder &&) = delete; - CborEncoder(Allocator *allocator) noexcept; + CborEncoder(Allocator *allocator = ApiAllocator()) noexcept; ~CborEncoder() noexcept; /** @@ -83,7 +83,7 @@ namespace Aws * * @param value value to encode. */ - void WriteUint(uint64_t value) noexcept; + void WriteUInt(uint64_t value) noexcept; /** * Encode a AWS_CBOR_TYPE_NEGINT value to "smallest possible" in encoder's buffer. @@ -223,7 +223,7 @@ namespace Aws * @param allocator * @param src The src data to decode from. */ - CborDecoder(Allocator *allocator, ByteCursor src) noexcept; + CborDecoder(ByteCursor src, Allocator *allocator = ApiAllocator()) noexcept; ~CborDecoder() noexcept; /** @@ -238,10 +238,11 @@ namespace Aws * Decode the next element and store it in the decoder cache if there was no element cached. * If there was an element cached, just return the type of the cached element. * - * @param out_type - * @return success/failure + * @return If successful, return the type of next element + * If not, return will be none and Aws::Crt::LastError() can be + * used to retrieve CRT error code. */ - bool PeekType(CborType &out_type) noexcept; + Optional PeekType() noexcept; /** * Consume the next data item, includes all the content within the data item. @@ -249,7 +250,7 @@ namespace Aws * As an example for the following CBOR, this function will consume all the data * as it's only one CBOR data item, an indefinite map with 2 key, value pair: * 0xbf6346756ef563416d7421ff - * BF -- Start indefinite-length map + * BF -- Start indefinite-length map * 63 -- First key, UTF-8 string length 3 * 46756e -- "Fun" * F5 -- First value, true @@ -260,7 +261,7 @@ namespace Aws * * Notes: this function will not ensure the data item is well-formed. * - * @return success/failure + * @return true if the operation succeed, false otherwise and LastError() will contain the errorCode. */ bool ConsumeNextWholeDataItem() noexcept; @@ -271,7 +272,7 @@ namespace Aws * 0xBF, "Start indefinite-length map", not any content of the map represented. * The next element to decode will start from 0x63. * 0xbf6346756ef563416d7421ff - * BF -- Start indefinite-length map + * BF -- Start indefinite-length map * 63 -- First key, UTF-8 string length 3 * 46756e -- "Fun" * F5 -- First value, true @@ -280,7 +281,7 @@ namespace Aws * 21 -- Second value, -2 * FF -- "break" * - * @return success/failure + * @return true if the operation succeed, false otherwise and LastError() will contain the errorCode. */ bool ConsumeNextSingleElement() noexcept; @@ -288,21 +289,22 @@ namespace Aws * Get the next element based on the type. If the next element doesn't match the expected type, an error * will be raised. If the next element has already been cached, it will consume the cached item when no * error was returned. Specifically: - * - Uint - PopNextUnsignedIntVal + * - UInt - PopNextUnsignedIntVal * - NegInt - PopNextNegativeIntVal, it represents (-1 - &out) * - Float - PopNextFloatVal * - Bytes - PopNextBytesVal * - Text - PopNextTextVal * - * @param out - * @return success/failure + * @return If successful, return the next element + * If not, return will be none and Aws::Crt::LastError() can be + * used to retrieve CRT error code. */ - bool PopNextUnsignedIntVal(uint64_t &out) noexcept; - bool PopNextNegativeIntVal(uint64_t &out) noexcept; - bool PopNextFloatVal(double &out) noexcept; - bool PopNextBooleanVal(bool &out) noexcept; - bool PopNextBytesVal(ByteCursor &out) noexcept; - bool PopNextTextVal(ByteCursor &out) noexcept; + Optional PopNextUnsignedIntVal() noexcept; + Optional PopNextNegativeIntVal() noexcept; + Optional PopNextFloatVal() noexcept; + Optional PopNextBooleanVal() noexcept; + Optional PopNextBytesVal() noexcept; + Optional PopNextTextVal() noexcept; /** * Get the next ArrayStart element. Only consume the ArrayStart element and set the size of array to @@ -315,10 +317,11 @@ namespace Aws * - Call ConsumeNextSingleElement to pop the indefinite-length start. * - Decode the next data item until Break is read. * - * @param out_size Store the size of array if succeeded. - * @return success/failure + * @return If successful, return the size of array + * If not, return will be none and Aws::Crt::LastError() can be + * used to retrieve CRT error code. */ - bool PopNextArrayStart(uint64_t &out_size) noexcept; + Optional PopNextArrayStart() noexcept; /** * Get the next MapStart element. Only consume the MapStart element and set the size of array to @@ -331,20 +334,22 @@ namespace Aws * - Call ConsumeNextSingleElement to pop the indefinite-length start. * - Decode the next data item until Break is read. * - * @param out_size Store the size of map if succeeded. - * @return success/failure + * @return If successful, return the size of map + * If not, return will be none and Aws::Crt::LastError() can be + * used to retrieve CRT error code. */ - bool PopNextMapStart(uint64_t &out_size) noexcept; + Optional PopNextMapStart() noexcept; /** * Get the next Tag element. Only consume the Tag element and set the tag value to out_tag_val, * not the content of the tagged value. The next CBOR data item will be the content of the tagged value * for a valid CBOR data. * - * @param out_tag_val Store the tag value if succeeded. - * @return success/failure + * @return If successful, return the tag value + * If not, return will be none and Aws::Crt::LastError() can be + * used to retrieve CRT error code. */ - bool PopNextTagVal(uint64_t &out_tag_val) noexcept; + Optional PopNextTagVal() noexcept; /** * @return the value of the last aws error encountered by operations on this instance. diff --git a/source/cbor/Cbor.cpp b/source/cbor/Cbor.cpp index e505dcb73..7061d4f4b 100644 --- a/source/cbor/Cbor.cpp +++ b/source/cbor/Cbor.cpp @@ -35,7 +35,7 @@ namespace Aws aws_cbor_encoder_reset(m_encoder); } - void CborEncoder::WriteUint(uint64_t value) noexcept + void CborEncoder::WriteUInt(uint64_t value) noexcept { aws_cbor_encoder_write_uint(m_encoder, value); } @@ -119,7 +119,7 @@ namespace Aws * CborDecoder * *****************************************************/ - CborDecoder::CborDecoder(Crt::Allocator *allocator, ByteCursor src) noexcept + CborDecoder::CborDecoder(ByteCursor src, Crt::Allocator *allocator) noexcept { m_decoder = aws_cbor_decoder_new(allocator, src); } @@ -134,16 +134,15 @@ namespace Aws return aws_cbor_decoder_get_remaining_length(m_decoder); } - bool CborDecoder::PeekType(CborType &out_type) noexcept + Optional CborDecoder::PeekType() noexcept { enum aws_cbor_type out_type_c = AWS_CBOR_TYPE_UNKNOWN; if (aws_cbor_decoder_peek_type(m_decoder, &out_type_c) != AWS_OP_SUCCESS) { m_lastError = aws_last_error(); - return false; + return Optional(); } - out_type = (CborType)out_type_c; - return true; + return Optional((CborType)out_type_c); } bool CborDecoder::ConsumeNextWholeDataItem() noexcept { @@ -165,94 +164,103 @@ namespace Aws return true; } - bool CborDecoder::PopNextUnsignedIntVal(uint64_t &out) noexcept + Optional CborDecoder::PopNextUnsignedIntVal() noexcept { + uint64_t out = 0; if (aws_cbor_decoder_pop_next_unsigned_int_val(m_decoder, &out) != AWS_OP_SUCCESS) { m_lastError = aws_last_error(); - return false; + return Optional(); } - return true; + return Optional(out); } - bool CborDecoder::PopNextNegativeIntVal(uint64_t &out) noexcept + Optional CborDecoder::PopNextNegativeIntVal() noexcept { + uint64_t out = 0; if (aws_cbor_decoder_pop_next_negative_int_val(m_decoder, &out) != AWS_OP_SUCCESS) { m_lastError = aws_last_error(); - return false; + return Optional(); } - return true; + return Optional(out); } - bool CborDecoder::PopNextFloatVal(double &out) noexcept + Optional CborDecoder::PopNextFloatVal() noexcept { + double out = 0; if (aws_cbor_decoder_pop_next_float_val(m_decoder, &out) != AWS_OP_SUCCESS) { m_lastError = aws_last_error(); - return false; + return Optional(); } - return true; + return Optional(out); } - bool CborDecoder::PopNextBooleanVal(bool &out) noexcept + Optional CborDecoder::PopNextBooleanVal() noexcept { + bool out = false; if (aws_cbor_decoder_pop_next_boolean_val(m_decoder, &out) != AWS_OP_SUCCESS) { m_lastError = aws_last_error(); - return false; + return Optional(); } - return true; + return Optional(out); } - bool CborDecoder::PopNextBytesVal(ByteCursor &out) noexcept + Optional CborDecoder::PopNextBytesVal() noexcept { + ByteCursor out = {0}; if (aws_cbor_decoder_pop_next_bytes_val(m_decoder, &out) != AWS_OP_SUCCESS) { m_lastError = aws_last_error(); - return false; + return Optional(); } - return true; + return Optional(out); } - bool CborDecoder::PopNextTextVal(ByteCursor &out) noexcept + Optional CborDecoder::PopNextTextVal() noexcept { + ByteCursor out = {0}; if (aws_cbor_decoder_pop_next_text_val(m_decoder, &out) != AWS_OP_SUCCESS) { m_lastError = aws_last_error(); - return false; + return Optional(); } - return true; + return Optional(out); } - bool CborDecoder::PopNextArrayStart(uint64_t &out_size) noexcept + Optional CborDecoder::PopNextArrayStart() noexcept { + uint64_t out_size = 0; if (aws_cbor_decoder_pop_next_array_start(m_decoder, &out_size) != AWS_OP_SUCCESS) { m_lastError = aws_last_error(); - return false; + return Optional(); } - return true; + return Optional(out_size); } - bool CborDecoder::PopNextMapStart(uint64_t &out_size) noexcept + Optional CborDecoder::PopNextMapStart() noexcept { + uint64_t out_size = 0; if (aws_cbor_decoder_pop_next_map_start(m_decoder, &out_size) != AWS_OP_SUCCESS) { m_lastError = aws_last_error(); - return false; + return Optional(); } - return true; + return Optional(out_size); } - bool CborDecoder::PopNextTagVal(uint64_t &out_tag_val) noexcept + Optional CborDecoder::PopNextTagVal() noexcept { + uint64_t out_tag_val = 0; if (aws_cbor_decoder_pop_next_tag_val(m_decoder, &out_tag_val) != AWS_OP_SUCCESS) { m_lastError = aws_last_error(); - return false; + return Optional(); } - return true; + return Optional(out_tag_val); } } // namespace Cbor diff --git a/tests/CborTest.cpp b/tests/CborTest.cpp index 80a979914..f6fa95370 100644 --- a/tests/CborTest.cpp +++ b/tests/CborTest.cpp @@ -29,7 +29,7 @@ static int s_CborSanityTest(struct aws_allocator *allocator, void *ctx) auto expected_tag_val = 999ULL; bool expected_bool_val = true; - encoder.WriteUint(expected_uint_val); + encoder.WriteUInt(expected_uint_val); encoder.WriteNegInt(expected_negint_val); encoder.WriteFloat(expected_float_val); encoder.WriteBytes(expected_bytes_val); @@ -48,80 +48,71 @@ static int s_CborSanityTest(struct aws_allocator *allocator, void *ctx) ByteCursor cursor = encoder.GetEncodedData(); - Cbor::CborDecoder decoder(allocator, cursor); - Cbor::CborType out_type = Cbor::CborType::Unknown; + Cbor::CborDecoder decoder(cursor, allocator); - // Check for Uint - uint64_t uint_val; - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::Uint); - ASSERT_TRUE(decoder.PopNextUnsignedIntVal(uint_val) && uint_val == expected_uint_val); + // Check for UInt + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::UInt); + ASSERT_TRUE(decoder.PopNextUnsignedIntVal().value() == expected_uint_val); // Check for NegInt - uint64_t negint_val; - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::NegInt); - ASSERT_TRUE(decoder.PopNextNegativeIntVal(negint_val) && negint_val == expected_negint_val); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::NegInt); + ASSERT_TRUE(decoder.PopNextNegativeIntVal().value() == expected_negint_val); // Check for Float - double float_val; - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::Float); - ASSERT_TRUE(decoder.PopNextFloatVal(float_val) && float_val == expected_float_val); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::Float); + ASSERT_TRUE(decoder.PopNextFloatVal().value() == expected_float_val); // Check for Bytes - ByteCursor bytes_val; - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::Bytes); - ASSERT_TRUE(decoder.PopNextBytesVal(bytes_val) && aws_byte_cursor_eq(&bytes_val, &expected_bytes_val)); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::Bytes); + ByteCursor decoded_val = decoder.PopNextBytesVal().value(); + ASSERT_TRUE(aws_byte_cursor_eq(&decoded_val, &expected_bytes_val)); // Check for Text - ByteCursor text_val; - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::Text); - ASSERT_TRUE(decoder.PopNextTextVal(text_val) && aws_byte_cursor_eq(&text_val, &expected_text_val)); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::Text); + decoded_val = decoder.PopNextTextVal().value(); + ASSERT_TRUE(aws_byte_cursor_eq(&decoded_val, &expected_text_val)); // Check for ArrayStart - uint64_t array_size; - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::ArrayStart); - ASSERT_TRUE(decoder.PopNextArrayStart(array_size) && array_size == expected_array_size); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::ArrayStart); + ASSERT_TRUE(decoder.PopNextArrayStart().value() == expected_array_size); // Check for MapStart - uint64_t map_size; - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::MapStart); - ASSERT_TRUE(decoder.PopNextMapStart(map_size) && map_size == expected_map_size); - + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::MapStart); + ASSERT_TRUE(decoder.PopNextMapStart().value() == expected_map_size); // Check for Tag - uint64_t tag_val; - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::Tag); - ASSERT_TRUE(decoder.PopNextTagVal(tag_val) && tag_val == expected_tag_val); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::Tag); + ASSERT_TRUE(decoder.PopNextTagVal().value() == expected_tag_val); // Check for Bool - bool bool_val; - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::Bool); - ASSERT_TRUE(decoder.PopNextBooleanVal(bool_val) && bool_val == expected_bool_val); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::Bool); + ASSERT_TRUE(decoder.PopNextBooleanVal().value() == expected_bool_val); // Check for Null - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::Null); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::Null); ASSERT_TRUE(decoder.ConsumeNextWholeDataItem()); // Check for Undefined - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::Undefined); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::Undefined); ASSERT_TRUE(decoder.ConsumeNextWholeDataItem()); // Check for Break - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::Break); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::Break); ASSERT_TRUE(decoder.ConsumeNextSingleElement()); // Check for IndefBytesStart - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::IndefBytesStart); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::IndefBytesStart); ASSERT_TRUE(decoder.ConsumeNextSingleElement()); // Check for IndefTextStart - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::IndefTextStart); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::IndefTextStart); ASSERT_TRUE(decoder.ConsumeNextSingleElement()); // Check for IndefArrayStart - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::IndefArrayStart); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::IndefArrayStart); ASSERT_TRUE(decoder.ConsumeNextSingleElement()); // Check for IndefMapStart - ASSERT_TRUE(decoder.PeekType(out_type) && out_type == Cbor::CborType::IndefMapStart); + ASSERT_TRUE(decoder.PeekType().value() == Cbor::CborType::IndefMapStart); ASSERT_TRUE(decoder.ConsumeNextSingleElement()); auto length = decoder.GetRemainingLength(); @@ -148,63 +139,59 @@ static void s_encode_timestamp_helper( static int s_decode_timestamp_helper(Cbor::CborDecoder &decoder, std::chrono::system_clock::time_point &outTimePoint) { - Cbor::CborType out_type = Cbor::CborType::Unknown; - if (!decoder.PeekType(out_type) && out_type != Cbor::CborType::Tag) + if (decoder.PeekType().value() != Cbor::CborType::Tag) { return AWS_OP_ERR; } - uint64_t tag_val = 0; - if (!decoder.PopNextTagVal(tag_val) && tag_val != AWS_CBOR_TAG_EPOCH_TIME) + if (decoder.PopNextTagVal().value() != AWS_CBOR_TAG_EPOCH_TIME) { return AWS_OP_ERR; } - if (decoder.PeekType(out_type)) + Cbor::CborType out_type = decoder.PeekType().value(); + switch (out_type) { - switch (out_type) + case Cbor::CborType::UInt: + case Cbor::CborType::NegInt: { - case Cbor::CborType::Uint: - case Cbor::CborType::NegInt: + int64_t secs_timestamp = 0; + uint64_t unsigned_val = 0; + if (out_type == Cbor::CborType::UInt) { - int64_t secs_timestamp = 0; - uint64_t unsigned_val = 0; - if (out_type == Cbor::CborType::Uint) - { - decoder.PopNextUnsignedIntVal(unsigned_val); - if (unsigned_val > INT64_MAX) - { - /* Overflow */ - return AWS_OP_ERR; - } - secs_timestamp = (int64_t)unsigned_val; - } - else + unsigned_val = decoder.PopNextUnsignedIntVal().value(); + if (unsigned_val > INT64_MAX) { - decoder.PopNextNegativeIntVal(unsigned_val); - if (unsigned_val > INT64_MAX) - { - /* Overflow */ - return AWS_OP_ERR; - } - /* convert the encoded number to real negative number */ - secs_timestamp = (int64_t)(-1 - unsigned_val); + /* Overflow */ + return AWS_OP_ERR; } - std::chrono::duration timestamp(secs_timestamp); - outTimePoint = std::chrono::system_clock::time_point(timestamp); - return AWS_OP_SUCCESS; + secs_timestamp = (int64_t)unsigned_val; } - case Cbor::CborType::Float: + else { - double double_val = 0; - decoder.PopNextFloatVal(double_val); - std::chrono::duration timestamp(double_val); - outTimePoint = std::chrono::system_clock::time_point( - std::chrono::duration_cast(timestamp)); - return AWS_OP_SUCCESS; + unsigned_val = decoder.PopNextNegativeIntVal().value(); + if (unsigned_val > INT64_MAX) + { + /* Overflow */ + return AWS_OP_ERR; + } + /* convert the encoded number to real negative number */ + secs_timestamp = (int64_t)(-1 - unsigned_val); } - default: - break; + std::chrono::duration timestamp(secs_timestamp); + outTimePoint = std::chrono::system_clock::time_point(timestamp); + return AWS_OP_SUCCESS; + } + case Cbor::CborType::Float: + { + double double_val = decoder.PopNextFloatVal().value(); + std::chrono::duration timestamp(double_val); + outTimePoint = + std::chrono::system_clock::time_point(std::chrono::duration_cast(timestamp)); + return AWS_OP_SUCCESS; } + default: + break; } + return AWS_OP_ERR; } @@ -239,7 +226,7 @@ static int s_CborTimeStampTest(struct aws_allocator *allocator, void *ctx) s_encode_timestamp_helper(encoder, now); ByteCursor cursor = encoder.GetEncodedData(); - Cbor::CborDecoder decoder(allocator, cursor); + Cbor::CborDecoder decoder(cursor); std::chrono::time_point decoded; ASSERT_SUCCESS(s_decode_timestamp_helper(decoder, decoded));