From a0eaaf3df3c00dfe0f373c3b4d80ca402f00f9cd Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 3 Jul 2025 16:10:42 +0200 Subject: [PATCH 1/6] add (u)int64 sentry_value_t type --- include/sentry.h | 22 +++++++ src/sentry_json.c | 20 +++++++ src/sentry_json.h | 10 ++++ src/sentry_value.c | 118 +++++++++++++++++++++++++++++++++++++ tests/unit/test_value.c | 127 +++++++++++++++++++++++++++++++++++++++- tests/unit/tests.inc | 2 + 6 files changed, 297 insertions(+), 2 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index 482e14751..0a9ccb62d 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -137,6 +137,8 @@ typedef enum { SENTRY_VALUE_TYPE_NULL, SENTRY_VALUE_TYPE_BOOL, SENTRY_VALUE_TYPE_INT32, + SENTRY_VALUE_TYPE_INT64, + SENTRY_VALUE_TYPE_UINT64, SENTRY_VALUE_TYPE_DOUBLE, SENTRY_VALUE_TYPE_STRING, SENTRY_VALUE_TYPE_LIST, @@ -200,6 +202,16 @@ SENTRY_API sentry_value_t sentry_value_new_null(void); */ SENTRY_API sentry_value_t sentry_value_new_int32(int32_t value); +/** + * Creates a new 64-bit signed integer value. + */ +SENTRY_API sentry_value_t sentry_value_new_int64(int64_t value); + +/** + * Creates a new 64-bit unsigned integer value. + */ +SENTRY_API sentry_value_t sentry_value_new_uint64(uint64_t value); + /** * Creates a new double value. */ @@ -338,6 +350,16 @@ SENTRY_API size_t sentry_value_get_length(sentry_value_t value); */ SENTRY_API int32_t sentry_value_as_int32(sentry_value_t value); +/** + * Converts a value into a 64 bit signed integer. + */ +SENTRY_API int64_t sentry_value_as_int64(sentry_value_t value); + +/** + * Converts a value into a 64 bit unsigned integer. + */ +SENTRY_API uint64_t sentry_value_as_uint64(sentry_value_t value); + /** * Converts a value into a double value. */ diff --git a/src/sentry_json.c b/src/sentry_json.c index 8902406f7..ea7454f81 100644 --- a/src/sentry_json.c +++ b/src/sentry_json.c @@ -364,6 +364,26 @@ sentry__jsonwriter_write_int32(sentry_jsonwriter_t *jw, int32_t val) } } +void +sentry__jsonwriter_write_int64(sentry_jsonwriter_t *jw, int64_t val) +{ + if (can_write_item(jw)) { + char buf[24]; + snprintf(buf, sizeof(buf), "%" PRId64, val); + write_str(jw, buf); + } +} + +void +sentry__jsonwriter_write_uint64(sentry_jsonwriter_t *jw, uint64_t val) +{ + if (can_write_item(jw)) { + char buf[24]; + snprintf(buf, sizeof(buf), "%" PRIu64, val); + write_str(jw, buf); + } +} + void sentry__jsonwriter_write_double(sentry_jsonwriter_t *jw, double val) { diff --git a/src/sentry_json.h b/src/sentry_json.h index 1467547c0..20f302518 100644 --- a/src/sentry_json.h +++ b/src/sentry_json.h @@ -53,6 +53,16 @@ void sentry__jsonwriter_write_bool(sentry_jsonwriter_t *jw, bool val); */ void sentry__jsonwriter_write_int32(sentry_jsonwriter_t *jw, int32_t val); +/** + * Write a 64-bit signed integer, encoded as JSON number. + */ +void sentry__jsonwriter_write_int64(sentry_jsonwriter_t *jw, int64_t val); + +/** + * Write a 64-bit unsigned integer, encoded as JSON number. + */ +void sentry__jsonwriter_write_uint64(sentry_jsonwriter_t *jw, uint64_t val); + /** * Write a 64-bit float, encoded as JSON number. */ diff --git a/src/sentry_value.c b/src/sentry_value.c index f47b5249b..31ce41efa 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -73,6 +73,8 @@ #define THING_TYPE_OBJECT 1 #define THING_TYPE_STRING 2 #define THING_TYPE_DOUBLE 3 +#define THING_TYPE_INT64 4 +#define THING_TYPE_UINT64 5 /* internal value helpers */ @@ -80,6 +82,8 @@ typedef struct { union { void *_ptr; double _double; + int64_t _int64; + uint64_t _uint64; } payload; long refcount; uint8_t type; @@ -326,6 +330,38 @@ sentry_value_new_double(double value) return rv; } +sentry_value_t +sentry_value_new_int64(int64_t value) +{ + thing_t *thing = sentry_malloc(sizeof(thing_t)); + if (!thing) { + return sentry_value_new_null(); + } + thing->payload._int64 = value; + thing->refcount = 1; + thing->type = (uint8_t)(THING_TYPE_INT64 | THING_TYPE_FROZEN); + + sentry_value_t rv; + rv._bits = (uint64_t)(size_t)thing; + return rv; +} + +sentry_value_t +sentry_value_new_uint64(uint64_t value) +{ + thing_t *thing = sentry_malloc(sizeof(thing_t)); + if (!thing) { + return sentry_value_new_null(); + } + thing->payload._uint64 = value; + thing->refcount = 1; + thing->type = (uint8_t)(THING_TYPE_UINT64 | THING_TYPE_FROZEN); + + sentry_value_t rv; + rv._bits = (uint64_t)(size_t)thing; + return rv; +} + sentry_value_t sentry_value_new_bool(int value) { @@ -490,6 +526,10 @@ sentry_value_get_type(sentry_value_t value) return SENTRY_VALUE_TYPE_OBJECT; case THING_TYPE_DOUBLE: return SENTRY_VALUE_TYPE_DOUBLE; + case THING_TYPE_INT64: + return SENTRY_VALUE_TYPE_INT64; + case THING_TYPE_UINT64: + return SENTRY_VALUE_TYPE_UINT64; } UNREACHABLE("invalid thing type"); } else if ((value._bits & TAG_MASK) == TAG_CONST) { @@ -648,6 +688,26 @@ sentry__value_stringify(sentry_value_t value) buf[written] = '\0'; return sentry__string_clone(buf); } + case SENTRY_VALUE_TYPE_INT64: { + char buf[24]; + size_t written = (size_t)sentry__snprintf_c( + buf, sizeof(buf), "%lld", (long long)sentry_value_as_int64(value)); + if (written >= sizeof(buf)) { + return sentry__string_clone(""); + } + buf[written] = '\0'; + return sentry__string_clone(buf); + } + case SENTRY_VALUE_TYPE_UINT64: { + char buf[24]; + size_t written = (size_t)sentry__snprintf_c(buf, sizeof(buf), "%llu", + (unsigned long long)sentry_value_as_uint64(value)); + if (written >= sizeof(buf)) { + return sentry__string_clone(""); + } + buf[written] = '\0'; + return sentry__string_clone(buf); + } } } @@ -681,6 +741,10 @@ sentry__value_clone(sentry_value_t value) case THING_TYPE_DOUBLE: sentry_value_incref(value); return value; + case THING_TYPE_INT64: + case THING_TYPE_UINT64: + sentry_value_incref(value); + return value; default: return sentry_value_new_null(); } @@ -893,6 +957,50 @@ sentry_value_as_double(sentry_value_t value) } } +int64_t +sentry_value_as_int64(sentry_value_t value) +{ + if ((value._bits & TAG_MASK) == TAG_INT32) { + return (int64_t)sentry_value_as_int32(value); + } + + const thing_t *thing = value_as_thing(value); + if (thing && thing_get_type(thing) == THING_TYPE_INT64) { + return thing->payload._int64; + } + if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { + return (int64_t)thing->payload._uint64; + } + if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { + return (int64_t)thing->payload._double; + } + return 0; +} + +uint64_t +sentry_value_as_uint64(sentry_value_t value) +{ + if ((value._bits & TAG_MASK) == TAG_INT32) { + int32_t int32_val = sentry_value_as_int32(value); + return int32_val >= 0 ? (uint64_t)int32_val : 0; + } + + const thing_t *thing = value_as_thing(value); + if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { + return thing->payload._uint64; + } + if (thing && thing_get_type(thing) == THING_TYPE_INT64) { + return thing->payload._int64 >= 0 ? (uint64_t)thing->payload._int64 : 0; + } + if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { + // TODO no check for double out of uint64 range + // TODO double gets truncated (1.9 -> 1) + return thing->payload._double >= 0 ? (uint64_t)thing->payload._double + : 0; + } + return 0; +} + const char * sentry_value_as_string(sentry_value_t value) { @@ -938,6 +1046,10 @@ sentry_value_is_true(sentry_value_t value) return 0; case SENTRY_VALUE_TYPE_INT32: return sentry_value_as_int32(value) != 0; + case SENTRY_VALUE_TYPE_INT64: + return sentry_value_as_int64(value) != 0; + case SENTRY_VALUE_TYPE_UINT64: + return sentry_value_as_uint64(value) != 0; case SENTRY_VALUE_TYPE_DOUBLE: return sentry_value_as_double(value) != 0.0; case SENTRY_VALUE_TYPE_STRING: @@ -1002,6 +1114,12 @@ sentry__jsonwriter_write_value(sentry_jsonwriter_t *jw, sentry_value_t value) case SENTRY_VALUE_TYPE_INT32: sentry__jsonwriter_write_int32(jw, sentry_value_as_int32(value)); break; + case SENTRY_VALUE_TYPE_INT64: + sentry__jsonwriter_write_int64(jw, sentry_value_as_int64(value)); + break; + case SENTRY_VALUE_TYPE_UINT64: + sentry__jsonwriter_write_uint64(jw, sentry_value_as_uint64(value)); + break; case SENTRY_VALUE_TYPE_DOUBLE: sentry__jsonwriter_write_double(jw, sentry_value_as_double(value)); break; diff --git a/tests/unit/test_value.c b/tests/unit/test_value.c index 40d0a2ae5..9ccd1854f 100644 --- a/tests/unit/test_value.c +++ b/tests/unit/test_value.c @@ -71,6 +71,129 @@ SENTRY_TEST(value_int32) TEST_CHECK(sentry_value_refcount(val) == 1); } +SENTRY_TEST(value_int64) +{ + sentry_value_t val = sentry_value_new_int64(42LL); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_INT64); + TEST_CHECK(sentry_value_as_int64(val) == 42LL); + // TODO only return NaN if outside of precision range? + TEST_CHECK(isnan(sentry_value_as_double(val))); + TEST_CHECK(sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "42"); + TEST_CHECK(sentry_value_refcount(val) == 1); + TEST_CHECK(sentry_value_is_frozen(val)); + sentry_value_decref(val); + + // Test large positive value + val = sentry_value_new_int64(INT64_MAX); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_INT64); + TEST_CHECK(sentry_value_as_int64(val) == INT64_MAX); + TEST_CHECK(sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "9223372036854775807"); + sentry_value_decref(val); + + // Test large negative value + val = sentry_value_new_int64(INT64_MIN); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_INT64); + TEST_CHECK(sentry_value_as_int64(val) == INT64_MIN); + TEST_CHECK(sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "-9223372036854775808"); + sentry_value_decref(val); + + // Test zero + val = sentry_value_new_int64(0LL); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_INT64); + TEST_CHECK(sentry_value_as_int64(val) == 0LL); + TEST_CHECK(!sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "0"); + TEST_CHECK(sentry_value_refcount(val) == 1); + TEST_CHECK(sentry_value_is_frozen(val)); + sentry_value_decref(val); + + // Test cross-type conversion + val = sentry_value_new_int32(42); + TEST_CHECK(sentry_value_as_int64(val) == 42LL); + sentry_value_decref(val); + + // Test double into uint64 + val = sentry_value_new_double(123.456); + TEST_CHECK(sentry_value_as_uint64(val) == 123ULL); + sentry_value_decref(val); + val = sentry_value_new_double(-123.456); + TEST_CHECK(sentry_value_as_int64(val) == -123ULL); + sentry_value_decref(val); + + // Test truncated double into uint64 + val = sentry_value_new_double(42.99); + TEST_CHECK(sentry_value_as_int64(val) == 42ULL); + sentry_value_decref(val); + val = sentry_value_new_double(-42.99); + TEST_CHECK(sentry_value_as_int64(val) == -42ULL); + sentry_value_decref(val); + + // Test large double reaching max uint64 + val = sentry_value_new_double(1.7976931348623157E+308); + TEST_CHECK(sentry_value_as_int64(val) == INT64_MAX); + sentry_value_decref(val); +} + +SENTRY_TEST(value_uint64) +{ + sentry_value_t val = sentry_value_new_uint64(42ULL); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_UINT64); + TEST_CHECK(sentry_value_as_uint64(val) == 42ULL); + // TODO only return NaN if outside of precision range? + TEST_CHECK(isnan(sentry_value_as_double(val))); + TEST_CHECK(sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "42"); + TEST_CHECK(sentry_value_refcount(val) == 1); + TEST_CHECK(sentry_value_is_frozen(val)); + sentry_value_decref(val); + + // Test large positive value + val = sentry_value_new_uint64(UINT64_MAX); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_UINT64); + TEST_CHECK(sentry_value_as_uint64(val) == UINT64_MAX); + TEST_CHECK(sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "18446744073709551615"); + sentry_value_decref(val); + + // Test zero + val = sentry_value_new_uint64(0ULL); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_UINT64); + TEST_CHECK(sentry_value_as_uint64(val) == 0ULL); + TEST_CHECK(!sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "0"); + TEST_CHECK(sentry_value_refcount(val) == 1); + TEST_CHECK(sentry_value_is_frozen(val)); + sentry_value_decref(val); + + // Test cross-type conversion + val = sentry_value_new_int32(42); + TEST_CHECK(sentry_value_as_uint64(val) == 42ULL); + sentry_value_decref(val); + + // Test negative int32 to uint64 conversion + val = sentry_value_new_int32(-42); + TEST_CHECK(sentry_value_as_uint64(val) == 0ULL); + sentry_value_decref(val); + + // Test double into uint64 + val = sentry_value_new_double(123.456); + TEST_CHECK(sentry_value_as_uint64(val) == 123ULL); + sentry_value_decref(val); + + // Test truncated double into uint64 + val = sentry_value_new_double(42.99); + TEST_CHECK(sentry_value_as_uint64(val) == 42ULL); + sentry_value_decref(val); + + // Test large double reaching max uint64 + val = sentry_value_new_double(1.7976931348623157E+308); + TEST_CHECK(sentry_value_as_uint64(val) == UINT64_MAX); + sentry_value_decref(val); +} + SENTRY_TEST(value_double) { sentry_value_t val = sentry_value_new_double(42.05); @@ -452,7 +575,7 @@ SENTRY_TEST(value_json_parsing) "\"surrogates\":\"𐐷\"}"); sentry_value_decref(rv); - // unmatched surrogates don’t parse + // unmatched surrogates don't parse rv = sentry__value_from_json(STRING("\"\\uD801\"")); TEST_CHECK(sentry_value_is_null(rv)); rv = sentry__value_from_json( @@ -521,7 +644,7 @@ SENTRY_TEST(value_json_surrogates) TEST_CHECK_JSON_VALUE(rv, "{\"surrogates\":\"oh 𐐷 hi\"}"); sentry_value_decref(rv); - // unmatched surrogates don’t parse + // unmatched surrogates don't parse rv = sentry__value_from_json(STRING("\"\\uD801\"")); TEST_CHECK(sentry_value_is_null(rv)); rv = sentry__value_from_json( diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 90ee30e8c..c836832b3 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -149,6 +149,7 @@ XX(value_double) XX(value_freezing) XX(value_get_by_null_key) XX(value_int32) +XX(value_int64) XX(value_json_deeply_nested) XX(value_json_escaping) XX(value_json_invalid_doubles) @@ -166,6 +167,7 @@ XX(value_set_by_null_key) XX(value_set_stacktrace) XX(value_string) XX(value_string_n) +XX(value_uint64) XX(value_unicode) XX(value_user) XX(value_wrong_type) From 118a632a1ad800bef81f1bd34b9a87a015637576 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 4 Jul 2025 11:18:47 +0200 Subject: [PATCH 2/6] add value_to_msgpack missing switch cases --- src/sentry_value.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/sentry_value.c b/src/sentry_value.c index 31ce41efa..307b01f1f 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -1172,6 +1172,12 @@ value_to_msgpack(mpack_writer_t *writer, sentry_value_t value) case SENTRY_VALUE_TYPE_INT32: mpack_write_i32(writer, sentry_value_as_int32(value)); break; + case SENTRY_VALUE_TYPE_INT64: + mpack_write_i64(writer, sentry_value_as_int64(value)); + break; + case SENTRY_VALUE_TYPE_UINT64: + mpack_write_u64(writer, sentry_value_as_uint64(value)); + break; case SENTRY_VALUE_TYPE_DOUBLE: mpack_write_double(writer, sentry_value_as_double(value)); break; From bcd93ec274210444b4b5b5324252e48e06ee83d2 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 4 Jul 2025 11:40:48 +0200 Subject: [PATCH 3/6] remove undefined behavior test (C99 6.3.1.4) --- tests/unit/test_value.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/unit/test_value.c b/tests/unit/test_value.c index 9ccd1854f..178911bf7 100644 --- a/tests/unit/test_value.c +++ b/tests/unit/test_value.c @@ -130,11 +130,6 @@ SENTRY_TEST(value_int64) val = sentry_value_new_double(-42.99); TEST_CHECK(sentry_value_as_int64(val) == -42ULL); sentry_value_decref(val); - - // Test large double reaching max uint64 - val = sentry_value_new_double(1.7976931348623157E+308); - TEST_CHECK(sentry_value_as_int64(val) == INT64_MAX); - sentry_value_decref(val); } SENTRY_TEST(value_uint64) @@ -187,11 +182,6 @@ SENTRY_TEST(value_uint64) val = sentry_value_new_double(42.99); TEST_CHECK(sentry_value_as_uint64(val) == 42ULL); sentry_value_decref(val); - - // Test large double reaching max uint64 - val = sentry_value_new_double(1.7976931348623157E+308); - TEST_CHECK(sentry_value_as_uint64(val) == UINT64_MAX); - sentry_value_decref(val); } SENTRY_TEST(value_double) From 8c7983c6641c11a5dc6f55c0db5a9fe785c90748 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 4 Jul 2025 13:04:55 +0200 Subject: [PATCH 4/6] avoid Windows sized integer name collision --- src/sentry_value.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/sentry_value.c b/src/sentry_value.c index 307b01f1f..3545f95d0 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -82,8 +82,8 @@ typedef struct { union { void *_ptr; double _double; - int64_t _int64; - uint64_t _uint64; + int64_t _int64_data; // to avoid shadowing __int64 (C2628 error) + uint64_t _uint64_data; } payload; long refcount; uint8_t type; @@ -337,7 +337,7 @@ sentry_value_new_int64(int64_t value) if (!thing) { return sentry_value_new_null(); } - thing->payload._int64 = value; + thing->payload._int64_data = value; thing->refcount = 1; thing->type = (uint8_t)(THING_TYPE_INT64 | THING_TYPE_FROZEN); @@ -353,7 +353,7 @@ sentry_value_new_uint64(uint64_t value) if (!thing) { return sentry_value_new_null(); } - thing->payload._uint64 = value; + thing->payload._uint64_data = value; thing->refcount = 1; thing->type = (uint8_t)(THING_TYPE_UINT64 | THING_TYPE_FROZEN); @@ -966,10 +966,10 @@ sentry_value_as_int64(sentry_value_t value) const thing_t *thing = value_as_thing(value); if (thing && thing_get_type(thing) == THING_TYPE_INT64) { - return thing->payload._int64; + return thing->payload._int64_data; } if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { - return (int64_t)thing->payload._uint64; + return (int64_t)thing->payload._uint64_data; } if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { return (int64_t)thing->payload._double; @@ -987,10 +987,12 @@ sentry_value_as_uint64(sentry_value_t value) const thing_t *thing = value_as_thing(value); if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { - return thing->payload._uint64; + return thing->payload._uint64_data; } if (thing && thing_get_type(thing) == THING_TYPE_INT64) { - return thing->payload._int64 >= 0 ? (uint64_t)thing->payload._int64 : 0; + return thing->payload._int64_data >= 0 + ? (uint64_t)thing->payload._int64_data + : 0; } if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { // TODO no check for double out of uint64 range From b7189eb536bdb7335e63f2e84955511d4a6b690a Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 8 Jul 2025 11:53:52 +0200 Subject: [PATCH 5/6] cleanup & apply code review feedback --- src/sentry_value.c | 26 +++++++++++--------------- tests/unit/test_value.c | 4 ++-- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/sentry_value.c b/src/sentry_value.c index 3545f95d0..233b35287 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -82,8 +82,8 @@ typedef struct { union { void *_ptr; double _double; - int64_t _int64_data; // to avoid shadowing __int64 (C2628 error) - uint64_t _uint64_data; + int64_t _i64; + uint64_t _u64; } payload; long refcount; uint8_t type; @@ -337,7 +337,7 @@ sentry_value_new_int64(int64_t value) if (!thing) { return sentry_value_new_null(); } - thing->payload._int64_data = value; + thing->payload._i64 = value; thing->refcount = 1; thing->type = (uint8_t)(THING_TYPE_INT64 | THING_TYPE_FROZEN); @@ -353,7 +353,7 @@ sentry_value_new_uint64(uint64_t value) if (!thing) { return sentry_value_new_null(); } - thing->payload._uint64_data = value; + thing->payload._u64 = value; thing->refcount = 1; thing->type = (uint8_t)(THING_TYPE_UINT64 | THING_TYPE_FROZEN); @@ -691,7 +691,7 @@ sentry__value_stringify(sentry_value_t value) case SENTRY_VALUE_TYPE_INT64: { char buf[24]; size_t written = (size_t)sentry__snprintf_c( - buf, sizeof(buf), "%lld", (long long)sentry_value_as_int64(value)); + buf, sizeof(buf), "%" PRIi64, sentry_value_as_int64(value)); if (written >= sizeof(buf)) { return sentry__string_clone(""); } @@ -700,8 +700,8 @@ sentry__value_stringify(sentry_value_t value) } case SENTRY_VALUE_TYPE_UINT64: { char buf[24]; - size_t written = (size_t)sentry__snprintf_c(buf, sizeof(buf), "%llu", - (unsigned long long)sentry_value_as_uint64(value)); + size_t written = (size_t)sentry__snprintf_c( + buf, sizeof(buf), "%" PRIi64, sentry_value_as_uint64(value)); if (written >= sizeof(buf)) { return sentry__string_clone(""); } @@ -739,8 +739,6 @@ sentry__value_clone(sentry_value_t value) } case THING_TYPE_STRING: case THING_TYPE_DOUBLE: - sentry_value_incref(value); - return value; case THING_TYPE_INT64: case THING_TYPE_UINT64: sentry_value_incref(value); @@ -966,10 +964,10 @@ sentry_value_as_int64(sentry_value_t value) const thing_t *thing = value_as_thing(value); if (thing && thing_get_type(thing) == THING_TYPE_INT64) { - return thing->payload._int64_data; + return thing->payload._i64; } if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { - return (int64_t)thing->payload._uint64_data; + return (int64_t)thing->payload._u64; } if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { return (int64_t)thing->payload._double; @@ -987,12 +985,10 @@ sentry_value_as_uint64(sentry_value_t value) const thing_t *thing = value_as_thing(value); if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { - return thing->payload._uint64_data; + return thing->payload._u64; } if (thing && thing_get_type(thing) == THING_TYPE_INT64) { - return thing->payload._int64_data >= 0 - ? (uint64_t)thing->payload._int64_data - : 0; + return thing->payload._i64 >= 0 ? (uint64_t)thing->payload._i64 : 0; } if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { // TODO no check for double out of uint64 range diff --git a/tests/unit/test_value.c b/tests/unit/test_value.c index 178911bf7..6c277daf7 100644 --- a/tests/unit/test_value.c +++ b/tests/unit/test_value.c @@ -117,13 +117,13 @@ SENTRY_TEST(value_int64) // Test double into uint64 val = sentry_value_new_double(123.456); - TEST_CHECK(sentry_value_as_uint64(val) == 123ULL); + TEST_CHECK(sentry_value_as_int64(val) == 123ULL); sentry_value_decref(val); val = sentry_value_new_double(-123.456); TEST_CHECK(sentry_value_as_int64(val) == -123ULL); sentry_value_decref(val); - // Test truncated double into uint64 + // Test truncated double into int64 val = sentry_value_new_double(42.99); TEST_CHECK(sentry_value_as_int64(val) == 42ULL); sentry_value_decref(val); From 84fb9665f527da73233879cf4363995660771c3a Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 8 Jul 2025 15:30:45 +0200 Subject: [PATCH 6/6] more cleanup & remove type coercion --- src/sentry_value.c | 91 +++++++++++++++++++++-------------------- tests/unit/test_value.c | 45 +++++++++----------- 2 files changed, 65 insertions(+), 71 deletions(-) diff --git a/src/sentry_value.c b/src/sentry_value.c index 233b35287..dcec55f3a 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -666,6 +666,18 @@ sentry__value_as_uuid(sentry_value_t value) char * sentry__value_stringify(sentry_value_t value) { +#define STRINGIFY_NUMERIC(fmt, value_fn) \ + do { \ + char buf[24]; \ + size_t written \ + = (size_t)sentry__snprintf_c(buf, sizeof(buf), fmt, value_fn); \ + if (written >= sizeof(buf)) { \ + return sentry__string_clone(""); \ + } \ + buf[written] = '\0'; \ + return sentry__string_clone(buf); \ + } while (0) + switch (sentry_value_get_type(value)) { case SENTRY_VALUE_TYPE_LIST: case SENTRY_VALUE_TYPE_OBJECT: @@ -676,39 +688,17 @@ sentry__value_stringify(sentry_value_t value) sentry_value_is_true(value) ? "true" : "false"); case SENTRY_VALUE_TYPE_STRING: return sentry__string_clone(sentry_value_as_string(value)); + case SENTRY_VALUE_TYPE_INT64: + STRINGIFY_NUMERIC("%" PRIi64, sentry_value_as_int64(value)); + case SENTRY_VALUE_TYPE_UINT64: + STRINGIFY_NUMERIC("%" PRIi64, sentry_value_as_uint64(value)); case SENTRY_VALUE_TYPE_INT32: case SENTRY_VALUE_TYPE_DOUBLE: - default: { - char buf[24]; - size_t written = (size_t)sentry__snprintf_c( - buf, sizeof(buf), "%g", sentry_value_as_double(value)); - if (written >= sizeof(buf)) { - return sentry__string_clone(""); - } - buf[written] = '\0'; - return sentry__string_clone(buf); - } - case SENTRY_VALUE_TYPE_INT64: { - char buf[24]; - size_t written = (size_t)sentry__snprintf_c( - buf, sizeof(buf), "%" PRIi64, sentry_value_as_int64(value)); - if (written >= sizeof(buf)) { - return sentry__string_clone(""); - } - buf[written] = '\0'; - return sentry__string_clone(buf); - } - case SENTRY_VALUE_TYPE_UINT64: { - char buf[24]; - size_t written = (size_t)sentry__snprintf_c( - buf, sizeof(buf), "%" PRIi64, sentry_value_as_uint64(value)); - if (written >= sizeof(buf)) { - return sentry__string_clone(""); - } - buf[written] = '\0'; - return sentry__string_clone(buf); - } + default: + STRINGIFY_NUMERIC("%g", sentry_value_as_double(value)); } + +#undef STRINGIFY_NUMERIC } sentry_value_t @@ -935,9 +925,18 @@ sentry_value_as_int32(sentry_value_t value) { if ((value._bits & TAG_MASK) == TAG_INT32) { return (int32_t)((int64_t)value._bits >> 32); - } else { - return 0; } + const thing_t *thing = value_as_thing(value); + if (thing && thing_get_type(thing) == THING_TYPE_INT64) { + SENTRY_WARN("Cannot convert int64 into int32, returning 0"); + } + if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { + SENTRY_WARN("Cannot convert uint64 into int32, returning 0"); + } + if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { + SENTRY_WARN("Cannot convert double into int32, returning 0"); + } + return 0; } double @@ -950,9 +949,15 @@ sentry_value_as_double(sentry_value_t value) const thing_t *thing = value_as_thing(value); if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { return thing->payload._double; - } else { - return (double)NAN; } + if (thing && thing_get_type(thing) == THING_TYPE_INT64) { + SENTRY_WARN("Cannot convert int64 into double, returning NAN"); + } + if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { + SENTRY_WARN("Cannot convert uint64 into double, returning NAN"); + } + + return (double)NAN; } int64_t @@ -967,10 +972,10 @@ sentry_value_as_int64(sentry_value_t value) return thing->payload._i64; } if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { - return (int64_t)thing->payload._u64; + SENTRY_WARN("Cannot convert uint64 into int64, returning 0"); } if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { - return (int64_t)thing->payload._double; + SENTRY_WARN("Cannot convert double into int64, returning 0"); } return 0; } @@ -979,8 +984,8 @@ uint64_t sentry_value_as_uint64(sentry_value_t value) { if ((value._bits & TAG_MASK) == TAG_INT32) { - int32_t int32_val = sentry_value_as_int32(value); - return int32_val >= 0 ? (uint64_t)int32_val : 0; + SENTRY_WARN("Cannot convert int32 into uint64, returning 0"); + return 0; } const thing_t *thing = value_as_thing(value); @@ -988,13 +993,10 @@ sentry_value_as_uint64(sentry_value_t value) return thing->payload._u64; } if (thing && thing_get_type(thing) == THING_TYPE_INT64) { - return thing->payload._i64 >= 0 ? (uint64_t)thing->payload._i64 : 0; + SENTRY_WARN("Cannot convert int64 into uint64, returning 0"); } if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { - // TODO no check for double out of uint64 range - // TODO double gets truncated (1.9 -> 1) - return thing->payload._double >= 0 ? (uint64_t)thing->payload._double - : 0; + SENTRY_WARN("Cannot convert int64 into uint64, returning 0"); } return 0; } @@ -1005,9 +1007,8 @@ sentry_value_as_string(sentry_value_t value) const thing_t *thing = value_as_thing(value); if (thing && thing_get_type(thing) == THING_TYPE_STRING) { return (const char *)thing->payload._ptr; - } else { - return ""; } + return ""; } sentry_value_t diff --git a/tests/unit/test_value.c b/tests/unit/test_value.c index 6c277daf7..18457c93b 100644 --- a/tests/unit/test_value.c +++ b/tests/unit/test_value.c @@ -76,8 +76,10 @@ SENTRY_TEST(value_int64) sentry_value_t val = sentry_value_new_int64(42LL); TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_INT64); TEST_CHECK(sentry_value_as_int64(val) == 42LL); - // TODO only return NaN if outside of precision range? + // We don't convert int64 to double TEST_CHECK(isnan(sentry_value_as_double(val))); + // We don't convert int64 to int32 + TEST_CHECK(sentry_value_as_int32(val) == 0); TEST_CHECK(sentry_value_is_true(val)); TEST_CHECK_JSON_VALUE(val, "42"); TEST_CHECK(sentry_value_refcount(val) == 1); @@ -110,25 +112,19 @@ SENTRY_TEST(value_int64) TEST_CHECK(sentry_value_is_frozen(val)); sentry_value_decref(val); - // Test cross-type conversion + // We do convert int32 to int64 val = sentry_value_new_int32(42); TEST_CHECK(sentry_value_as_int64(val) == 42LL); sentry_value_decref(val); - // Test double into uint64 - val = sentry_value_new_double(123.456); - TEST_CHECK(sentry_value_as_int64(val) == 123ULL); - sentry_value_decref(val); - val = sentry_value_new_double(-123.456); - TEST_CHECK(sentry_value_as_int64(val) == -123ULL); + // We don't convert uint64 to int64 + val = sentry_value_new_uint64(-42LL); + TEST_CHECK(sentry_value_as_int64(val) == 0); sentry_value_decref(val); - // Test truncated double into int64 + // We don't convert double to int64 val = sentry_value_new_double(42.99); - TEST_CHECK(sentry_value_as_int64(val) == 42ULL); - sentry_value_decref(val); - val = sentry_value_new_double(-42.99); - TEST_CHECK(sentry_value_as_int64(val) == -42ULL); + TEST_CHECK(sentry_value_as_int64(val) == 0); sentry_value_decref(val); } @@ -137,8 +133,10 @@ SENTRY_TEST(value_uint64) sentry_value_t val = sentry_value_new_uint64(42ULL); TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_UINT64); TEST_CHECK(sentry_value_as_uint64(val) == 42ULL); - // TODO only return NaN if outside of precision range? + // We don't convert uint64 to double TEST_CHECK(isnan(sentry_value_as_double(val))); + // We don't convert uint64 to int32 + TEST_CHECK(sentry_value_as_int32(val) == 0); TEST_CHECK(sentry_value_is_true(val)); TEST_CHECK_JSON_VALUE(val, "42"); TEST_CHECK(sentry_value_refcount(val) == 1); @@ -163,24 +161,19 @@ SENTRY_TEST(value_uint64) TEST_CHECK(sentry_value_is_frozen(val)); sentry_value_decref(val); - // Test cross-type conversion + // We don't convert int32 to uint64 val = sentry_value_new_int32(42); - TEST_CHECK(sentry_value_as_uint64(val) == 42ULL); + TEST_CHECK(sentry_value_as_uint64(val) == 0); sentry_value_decref(val); - // Test negative int32 to uint64 conversion - val = sentry_value_new_int32(-42); - TEST_CHECK(sentry_value_as_uint64(val) == 0ULL); - sentry_value_decref(val); - - // Test double into uint64 + // We don't convert double to uint64 val = sentry_value_new_double(123.456); - TEST_CHECK(sentry_value_as_uint64(val) == 123ULL); + TEST_CHECK(sentry_value_as_uint64(val) == 0); sentry_value_decref(val); - // Test truncated double into uint64 - val = sentry_value_new_double(42.99); - TEST_CHECK(sentry_value_as_uint64(val) == 42ULL); + // We don't convert int64 to uint64 + val = sentry_value_new_int64(42LL); + TEST_CHECK(sentry_value_as_uint64(val) == 0); sentry_value_decref(val); }