From 8340b0111ce0f6080498064a369ae968a3044b6c Mon Sep 17 00:00:00 2001 From: Pavel Kalvoda Date: Sat, 3 Jun 2023 15:32:47 +0200 Subject: [PATCH 1/4] Update cbor_describe after #281 --- src/cbor.c | 2 +- test/pretty_printer_test.c | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/cbor.c b/src/cbor.c index 1de49cd..61209a1 100644 --- a/src/cbor.c +++ b/src/cbor.c @@ -329,7 +329,7 @@ static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) { _cbor_nested_describe(cbor_bytestring_chunks_handle(item)[i], out, indent + 4); } else { - const unsigned char* data = cbor_bytestring_handle(item); + const unsigned char *data = cbor_bytestring_handle(item); fprintf(out, "Definite, length %zuB\n", cbor_bytestring_length(item)); fprintf(out, "%*s", indent + 4, " "); for (size_t i = 0; i < cbor_bytestring_length(item); i++) diff --git a/test/pretty_printer_test.c b/test/pretty_printer_test.c index f77f029..e1fc857 100644 --- a/test/pretty_printer_test.c +++ b/test/pretty_printer_test.c @@ -55,8 +55,9 @@ static void test_definite_bytestring(void **_CBOR_UNUSED(_state)) { cbor_item_t *item = cbor_build_bytestring(data, 3); char *expected[] = { "[CBOR_TYPE_BYTESTRING] Definite, length 3B", + " 010203", }; - assert_describe_result(item, expected, 1); + assert_describe_result(item, expected, 2); cbor_decref(&item); } @@ -70,9 +71,11 @@ static void test_indefinite_bytestring(void **_CBOR_UNUSED(_state)) { char *expected[] = { "[CBOR_TYPE_BYTESTRING] Indefinite, with 2 chunks:", " [CBOR_TYPE_BYTESTRING] Definite, length 3B", + " 010203", " [CBOR_TYPE_BYTESTRING] Definite, length 2B", + " 0102", }; - assert_describe_result(item, expected, 3); + assert_describe_result(item, expected, 5); cbor_decref(&item); } From 2e729fdd8c4b9ff11423f7c74aea03fde8dccf4b Mon Sep 17 00:00:00 2001 From: Pavel Kalvoda Date: Sat, 3 Jun 2023 16:12:19 +0200 Subject: [PATCH 2/4] Improve string handling in cbor_describe --- src/cbor.c | 10 ++++------ test/pretty_printer_test.c | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/cbor.c b/src/cbor.c index 61209a1..28a720c 100644 --- a/src/cbor.c +++ b/src/cbor.c @@ -349,13 +349,11 @@ static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) { } else { fprintf(out, "Definite, length %zuB, %zu codepoints\n", cbor_string_length(item), cbor_string_codepoint_count(item)); - /* Careful - this doesn't support multibyte characters! */ - /* Printing those is out of the scope of this demo :) */ - /* libICU is your friend */ - // TODO: Handle escaping fprintf(out, "%*s", indent + 4, " "); - /* XXX: no null at the end -> confused vprintf */ - fwrite(cbor_string_handle(item), (int)cbor_string_length(item), 1, out); + // Note: The string is not escaped, whitespace and control character + // will be printed in verbatim and take effect. + fwrite(cbor_string_handle(item), sizeof(unsigned char), + cbor_string_length(item), out); fprintf(out, "\n"); } break; diff --git a/test/pretty_printer_test.c b/test/pretty_printer_test.c index e1fc857..7b259ce 100644 --- a/test/pretty_printer_test.c +++ b/test/pretty_printer_test.c @@ -109,6 +109,20 @@ static void test_indefinite_string(void **_CBOR_UNUSED(_state)) { cbor_decref(&item); } +static void test_multibyte_string(void **_CBOR_UNUSED(_state)) { + // "Štěstíčko" in UTF-8 + char *string = "\xc5\xa0t\xc4\x9bst\xc3\xad\xc4\x8dko"; + cbor_item_t *item = cbor_build_string(string); + cbor_describe(item, stdout); + char *expected[] = { + // TODO: Codepoint metadata is not set by the builder, fix. + "[CBOR_TYPE_STRING] Definite, length 13B, 0 codepoints", + " \xc5\xa0t\xc4\x9bst\xc3\xad\xc4\x8dko", + }; + assert_describe_result(item, expected, 2); + cbor_decref(&item); +} + static void test_definite_array(void **_CBOR_UNUSED(_state)) { cbor_item_t *item = cbor_new_definite_array(2); assert_true(cbor_array_push(item, cbor_move(cbor_build_uint8(1)))); @@ -202,6 +216,7 @@ int main(void) { cmocka_unit_test(test_indefinite_bytestring), cmocka_unit_test(test_definite_string), cmocka_unit_test(test_indefinite_string), + cmocka_unit_test(test_multibyte_string), cmocka_unit_test(test_definite_array), cmocka_unit_test(test_indefinite_array), cmocka_unit_test(test_definite_map), From 9730caa640ae08340dfadeaf4837c91acc4ec36e Mon Sep 17 00:00:00 2001 From: Pavel Kalvoda Date: Sat, 3 Jun 2023 18:47:29 +0200 Subject: [PATCH 3/4] Improve cbor_describe consistency and simplify tests --- src/cbor.c | 43 ++++++---- test/pretty_printer_test.c | 164 ++++++++++++++++--------------------- 2 files changed, 97 insertions(+), 110 deletions(-) diff --git a/src/cbor.c b/src/cbor.c index 28a720c..a8b4bcd 100644 --- a/src/cbor.c +++ b/src/cbor.c @@ -306,7 +306,7 @@ static void _cbor_type_marquee(FILE *out, char *label, int indent) { } static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) { - setlocale(LC_ALL, ""); + const int indent_offset = 4; switch (cbor_typeof(item)) { case CBOR_TYPE_UINT: { _cbor_type_marquee(out, "CBOR_TYPE_UINT", indent); @@ -323,15 +323,16 @@ static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) { case CBOR_TYPE_BYTESTRING: { _cbor_type_marquee(out, "CBOR_TYPE_BYTESTRING", indent); if (cbor_bytestring_is_indefinite(item)) { - fprintf(out, "Indefinite, with %zu chunks:\n", + fprintf(out, "Indefinite, Chunks: %zu, Chunk data:\n", cbor_bytestring_chunk_count(item)); for (size_t i = 0; i < cbor_bytestring_chunk_count(item); i++) _cbor_nested_describe(cbor_bytestring_chunks_handle(item)[i], out, - indent + 4); + indent + indent_offset); } else { const unsigned char *data = cbor_bytestring_handle(item); - fprintf(out, "Definite, length %zuB\n", cbor_bytestring_length(item)); - fprintf(out, "%*s", indent + 4, " "); + fprintf(out, "Definite, Length: %zuB, Data:\n", + cbor_bytestring_length(item)); + fprintf(out, "%*s", indent + indent_offset, " "); for (size_t i = 0; i < cbor_bytestring_length(item); i++) fprintf(out, "%02x", (int)(data[i] & 0xff)); fprintf(out, "\n"); @@ -341,15 +342,15 @@ static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) { case CBOR_TYPE_STRING: { _cbor_type_marquee(out, "CBOR_TYPE_STRING", indent); if (cbor_string_is_indefinite(item)) { - fprintf(out, "Indefinite, with %zu chunks:\n", + fprintf(out, "Indefinite, Chunks: %zu, Chunk data:\n", cbor_string_chunk_count(item)); for (size_t i = 0; i < cbor_string_chunk_count(item); i++) _cbor_nested_describe(cbor_string_chunks_handle(item)[i], out, - indent + 4); + indent + indent_offset); } else { - fprintf(out, "Definite, length %zuB, %zu codepoints\n", + fprintf(out, "Definite, Length: %zuB, Codepoints: %zu, Data:\n", cbor_string_length(item), cbor_string_codepoint_count(item)); - fprintf(out, "%*s", indent + 4, " "); + fprintf(out, "%*s", indent + indent_offset, " "); // Note: The string is not escaped, whitespace and control character // will be printed in verbatim and take effect. fwrite(cbor_string_handle(item), sizeof(unsigned char), @@ -361,34 +362,40 @@ static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) { case CBOR_TYPE_ARRAY: { _cbor_type_marquee(out, "CBOR_TYPE_ARRAY", indent); if (cbor_array_is_definite(item)) { - fprintf(out, "Definite, size: %zu\n", cbor_array_size(item)); + fprintf(out, "Definite, Size: %zu, Contents:\n", cbor_array_size(item)); } else { - fprintf(out, "Indefinite, size: %zu\n", cbor_array_size(item)); + fprintf(out, "Indefinite, Size: %zu, Contents:\n", + cbor_array_size(item)); } for (size_t i = 0; i < cbor_array_size(item); i++) - _cbor_nested_describe(cbor_array_handle(item)[i], out, indent + 4); + _cbor_nested_describe(cbor_array_handle(item)[i], out, + indent + indent_offset); break; } case CBOR_TYPE_MAP: { _cbor_type_marquee(out, "CBOR_TYPE_MAP", indent); if (cbor_map_is_definite(item)) { - fprintf(out, "Definite, size: %zu\n", cbor_map_size(item)); + fprintf(out, "Definite, Size: %zu, Contents:\n", cbor_map_size(item)); } else { - fprintf(out, "Indefinite, size: %zu\n", cbor_map_size(item)); + fprintf(out, "Indefinite, Size: %zu, Contents:\n", cbor_map_size(item)); } // TODO: Label and group keys and values for (size_t i = 0; i < cbor_map_size(item); i++) { - _cbor_nested_describe(cbor_map_handle(item)[i].key, out, indent + 4); - _cbor_nested_describe(cbor_map_handle(item)[i].value, out, indent + 4); + fprintf(out, "%*sMap entry %zu\n", indent + indent_offset, " ", i); + _cbor_nested_describe(cbor_map_handle(item)[i].key, out, + indent + 2 * indent_offset); + _cbor_nested_describe(cbor_map_handle(item)[i].value, out, + indent + 2 * indent_offset); } break; } case CBOR_TYPE_TAG: { _cbor_type_marquee(out, "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); + _cbor_nested_describe(cbor_move(cbor_tag_item(item)), out, + indent + indent_offset); break; } case CBOR_TYPE_FLOAT_CTRL: { @@ -404,7 +411,7 @@ static void _cbor_nested_describe(cbor_item_t *item, FILE *out, int indent) { fprintf(out, "Simple value: %d\n", cbor_ctrl_value(item)); } else { fprintf(out, "Width: %dB, ", _pow(2, cbor_float_get_width(item))); - fprintf(out, "value: %lf\n", cbor_float_get_float(item)); + fprintf(out, "Value: %lf\n", cbor_float_get_float(item)); } break; } diff --git a/test/pretty_printer_test.c b/test/pretty_printer_test.c index 7b259ce..6c341cb 100644 --- a/test/pretty_printer_test.c +++ b/test/pretty_printer_test.c @@ -11,53 +11,46 @@ #include "assertions.h" #include "cbor.h" -void assert_describe_result(cbor_item_t *item, char *result[], size_t lines) { +void assert_describe_result(cbor_item_t *item, char *expected_result) { #if CBOR_PRETTY_PRINTER + // We know the expected size based on `expected_result`, but read everything + // in order to get the full actual output in a useful error message. + const size_t buffer_size = 512; FILE *outfile = tmpfile(); cbor_describe(item, outfile); rewind(outfile); - for (size_t i = 0; i < lines; i++) { - // Expected line + linebreak + null character - size_t buffer_size = strlen(result[i]) + 2; - char *buffer = malloc(buffer_size); - char *result_with_newline = malloc(buffer_size); - assert_non_null(buffer); - assert_non_null(result_with_newline); - snprintf(result_with_newline, buffer_size, "%s\n", result[i]); - fgets(buffer, strlen(result[i]) + 2, outfile); - assert_int_equal(strlen(buffer), strlen(result_with_newline)); - assert_string_equal(buffer, result_with_newline); - free(buffer); - free(result_with_newline); - } - fgetc(outfile); + // Treat string as null-terminated since cmocka doesn't have asserts + // for explicit length strings. + char *output = malloc(buffer_size); + assert_non_null(output); + size_t output_size = fread(output, sizeof(char), buffer_size, outfile); + output[output_size] = '\0'; + assert_string_equal(output, expected_result); assert_true(feof(outfile)); + free(output); fclose(outfile); #endif } static void test_uint(void **_CBOR_UNUSED(_state)) { cbor_item_t *item = cbor_build_uint8(42); - char *expected[] = {"[CBOR_TYPE_UINT] Width: 1B, Value: 42"}; - assert_describe_result(item, expected, 1); + assert_describe_result(item, "[CBOR_TYPE_UINT] Width: 1B, Value: 42\n"); cbor_decref(&item); } static void test_negint(void **_CBOR_UNUSED(_state)) { cbor_item_t *item = cbor_build_negint16(40); - char *expected[] = {"[CBOR_TYPE_NEGINT] Width: 2B, Value: -40 - 1"}; - assert_describe_result(item, expected, 1); + assert_describe_result(item, + "[CBOR_TYPE_NEGINT] Width: 2B, Value: -40 - 1\n"); cbor_decref(&item); } static void test_definite_bytestring(void **_CBOR_UNUSED(_state)) { unsigned char data[] = {0x01, 0x02, 0x03}; cbor_item_t *item = cbor_build_bytestring(data, 3); - char *expected[] = { - "[CBOR_TYPE_BYTESTRING] Definite, length 3B", - " 010203", - }; - assert_describe_result(item, expected, 2); + assert_describe_result(item, + "[CBOR_TYPE_BYTESTRING] Definite, Length: 3B, Data:\n" + " 010203\n"); cbor_decref(&item); } @@ -68,26 +61,24 @@ static void test_indefinite_bytestring(void **_CBOR_UNUSED(_state)) { item, cbor_move(cbor_build_bytestring(data, 3)))); assert_true(cbor_bytestring_add_chunk( item, cbor_move(cbor_build_bytestring(data, 2)))); - char *expected[] = { - "[CBOR_TYPE_BYTESTRING] Indefinite, with 2 chunks:", - " [CBOR_TYPE_BYTESTRING] Definite, length 3B", - " 010203", - " [CBOR_TYPE_BYTESTRING] Definite, length 2B", - " 0102", - }; - assert_describe_result(item, expected, 5); + assert_describe_result( + item, + "[CBOR_TYPE_BYTESTRING] Indefinite, Chunks: 2, Chunk data:\n" + " [CBOR_TYPE_BYTESTRING] Definite, Length: 3B, Data:\n" + " 010203\n" + " [CBOR_TYPE_BYTESTRING] Definite, Length: 2B, Data:\n" + " 0102\n"); cbor_decref(&item); } static void test_definite_string(void **_CBOR_UNUSED(_state)) { char *string = "Hello!"; cbor_item_t *item = cbor_build_string(string); - char *expected[] = { - // TODO: Codepoint metadata is not set by the builder, fix. - "[CBOR_TYPE_STRING] Definite, length 6B, 0 codepoints", - " Hello!", - }; - assert_describe_result(item, expected, 2); + // TODO: Codepoint metadata is not set by the builder, fix. + assert_describe_result( + item, + "[CBOR_TYPE_STRING] Definite, Length: 6B, Codepoints: 0, Data:\n" + " Hello!\n"); cbor_decref(&item); } @@ -98,14 +89,13 @@ static void test_indefinite_string(void **_CBOR_UNUSED(_state)) { cbor_string_add_chunk(item, cbor_move(cbor_build_string(string)))); assert_true( cbor_string_add_chunk(item, cbor_move(cbor_build_string(string)))); - char *expected[] = { - "[CBOR_TYPE_STRING] Indefinite, with 2 chunks:", - " [CBOR_TYPE_STRING] Definite, length 6B, 0 codepoints", - " Hello!", - " [CBOR_TYPE_STRING] Definite, length 6B, 0 codepoints", - " Hello!", - }; - assert_describe_result(item, expected, 5); + assert_describe_result( + item, + "[CBOR_TYPE_STRING] Indefinite, Chunks: 2, Chunk data:\n" + " [CBOR_TYPE_STRING] Definite, Length: 6B, Codepoints: 0, Data:\n" + " Hello!\n" + " [CBOR_TYPE_STRING] Definite, Length: 6B, Codepoints: 0, Data:\n" + " Hello!\n"); cbor_decref(&item); } @@ -113,13 +103,11 @@ static void test_multibyte_string(void **_CBOR_UNUSED(_state)) { // "Štěstíčko" in UTF-8 char *string = "\xc5\xa0t\xc4\x9bst\xc3\xad\xc4\x8dko"; cbor_item_t *item = cbor_build_string(string); - cbor_describe(item, stdout); - char *expected[] = { - // TODO: Codepoint metadata is not set by the builder, fix. - "[CBOR_TYPE_STRING] Definite, length 13B, 0 codepoints", - " \xc5\xa0t\xc4\x9bst\xc3\xad\xc4\x8dko", - }; - assert_describe_result(item, expected, 2); + // TODO: Codepoint metadata is not set by the builder, fix. + assert_describe_result( + item, + "[CBOR_TYPE_STRING] Definite, Length: 13B, Codepoints: 0, Data:\n" + " \xc5\xa0t\xc4\x9bst\xc3\xad\xc4\x8dko\n"); cbor_decref(&item); } @@ -127,12 +115,10 @@ static void test_definite_array(void **_CBOR_UNUSED(_state)) { cbor_item_t *item = cbor_new_definite_array(2); assert_true(cbor_array_push(item, cbor_move(cbor_build_uint8(1)))); assert_true(cbor_array_push(item, cbor_move(cbor_build_uint8(2)))); - char *expected[] = { - "[CBOR_TYPE_ARRAY] Definite, size: 2", - " [CBOR_TYPE_UINT] Width: 1B, Value: 1", - " [CBOR_TYPE_UINT] Width: 1B, Value: 2", - }; - assert_describe_result(item, expected, 3); + assert_describe_result(item, + "[CBOR_TYPE_ARRAY] Definite, Size: 2, Contents:\n" + " [CBOR_TYPE_UINT] Width: 1B, Value: 1\n" + " [CBOR_TYPE_UINT] Width: 1B, Value: 2\n"); cbor_decref(&item); } @@ -140,12 +126,10 @@ static void test_indefinite_array(void **_CBOR_UNUSED(_state)) { cbor_item_t *item = cbor_new_indefinite_array(); assert_true(cbor_array_push(item, cbor_move(cbor_build_uint8(1)))); assert_true(cbor_array_push(item, cbor_move(cbor_build_uint8(2)))); - char *expected[] = { - "[CBOR_TYPE_ARRAY] Indefinite, size: 2", - " [CBOR_TYPE_UINT] Width: 1B, Value: 1", - " [CBOR_TYPE_UINT] Width: 1B, Value: 2", - }; - assert_describe_result(item, expected, 3); + assert_describe_result(item, + "[CBOR_TYPE_ARRAY] Indefinite, Size: 2, Contents:\n" + " [CBOR_TYPE_UINT] Width: 1B, Value: 1\n" + " [CBOR_TYPE_UINT] Width: 1B, Value: 2\n"); cbor_decref(&item); } @@ -154,12 +138,12 @@ static void test_definite_map(void **_CBOR_UNUSED(_state)) { assert_true(cbor_map_add( item, (struct cbor_pair){.key = cbor_move(cbor_build_uint8(1)), .value = cbor_move(cbor_build_uint8(2))})); - char *expected[] = { - "[CBOR_TYPE_MAP] Definite, size: 1", - " [CBOR_TYPE_UINT] Width: 1B, Value: 1", - " [CBOR_TYPE_UINT] Width: 1B, Value: 2", - }; - assert_describe_result(item, expected, 3); + cbor_describe(item, stdout); + assert_describe_result(item, + "[CBOR_TYPE_MAP] Definite, Size: 1, Contents:\n" + " Map entry 0\n" + " [CBOR_TYPE_UINT] Width: 1B, Value: 1\n" + " [CBOR_TYPE_UINT] Width: 1B, Value: 2\n"); cbor_decref(&item); } @@ -168,22 +152,19 @@ static void test_indefinite_map(void **_CBOR_UNUSED(_state)) { assert_true(cbor_map_add( item, (struct cbor_pair){.key = cbor_move(cbor_build_uint8(1)), .value = cbor_move(cbor_build_uint8(2))})); - char *expected[] = { - "[CBOR_TYPE_MAP] Indefinite, size: 1", - " [CBOR_TYPE_UINT] Width: 1B, Value: 1", - " [CBOR_TYPE_UINT] Width: 1B, Value: 2", - }; - assert_describe_result(item, expected, 3); + assert_describe_result(item, + "[CBOR_TYPE_MAP] Indefinite, Size: 1, Contents:\n" + " Map entry 0\n" + " [CBOR_TYPE_UINT] Width: 1B, Value: 1\n" + " [CBOR_TYPE_UINT] Width: 1B, Value: 2\n"); cbor_decref(&item); } static void test_tag(void **_CBOR_UNUSED(_state)) { cbor_item_t *item = cbor_build_tag(42, cbor_move(cbor_build_uint8(1))); - char *expected[] = { - "[CBOR_TYPE_TAG] Value: 42", - " [CBOR_TYPE_UINT] Width: 1B, Value: 1", - }; - assert_describe_result(item, expected, 2); + assert_describe_result(item, + "[CBOR_TYPE_TAG] Value: 42\n" + " [CBOR_TYPE_UINT] Width: 1B, Value: 1\n"); cbor_decref(&item); } @@ -196,15 +177,14 @@ static void test_floats(void **_CBOR_UNUSED(_state)) { cbor_array_push(item, cbor_move(cbor_build_ctrl(CBOR_CTRL_NULL)))); assert_true(cbor_array_push(item, cbor_move(cbor_build_ctrl(24)))); assert_true(cbor_array_push(item, cbor_move(cbor_build_float4(3.14f)))); - char *expected[] = { - "[CBOR_TYPE_ARRAY] Indefinite, size: 5", - " [CBOR_TYPE_FLOAT_CTRL] Bool: true", - " [CBOR_TYPE_FLOAT_CTRL] Undefined", - " [CBOR_TYPE_FLOAT_CTRL] Null", - " [CBOR_TYPE_FLOAT_CTRL] Simple value: 24", - " [CBOR_TYPE_FLOAT_CTRL] Width: 4B, value: 3.140000", - }; - assert_describe_result(item, expected, 6); + assert_describe_result( + item, + "[CBOR_TYPE_ARRAY] Indefinite, Size: 5, Contents:\n" + " [CBOR_TYPE_FLOAT_CTRL] Bool: true\n" + " [CBOR_TYPE_FLOAT_CTRL] Undefined\n" + " [CBOR_TYPE_FLOAT_CTRL] Null\n" + " [CBOR_TYPE_FLOAT_CTRL] Simple value: 24\n" + " [CBOR_TYPE_FLOAT_CTRL] Width: 4B, Value: 3.140000\n"); cbor_decref(&item); } From aa82657b82f42d2df40b0d328c6e89074b59340a Mon Sep 17 00:00:00 2001 From: Pavel Kalvoda Date: Sat, 3 Jun 2023 18:51:55 +0200 Subject: [PATCH 4/4] Update changelog for describe changes --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae54670..836de88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ Template: Next --------------------- - [Updated documentation to refer to RFC 8949](https://github.com/PJK/libcbor/issues/269) +- Improvements to `cbor_describe` + - [Bytestring data will now be printed as well](https://github.com/PJK/libcbor/pull/281) by [akallabeth](https://github.com/akallabeth) + - [Formatting consistency and clarity improvements](https://github.com/PJK/libcbor/pull/285) 0.10.2 (2023-01-31) ---------------------