apply suggestions from code review

JoshuaMoelans · JoshuaMoelans · commit e6ea228bfe86 · 2025-07-15T15:14:30.000+02:00
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -15,6 +15,8 @@ sentry_target_sources_cwd(sentry
 	sentry_json.h
 	sentry_logger.c
 	sentry_logger.h
+	sentry_logs.c
+	sentry_logs.h
 	sentry_options.c
 	sentry_options.h
 	sentry_os.c
@@ -51,8 +53,6 @@ sentry_target_sources_cwd(sentry
 	transports/sentry_disk_transport.h
 	transports/sentry_function_transport.c
 	unwinder/sentry_unwinder.c
-	sentry_logs.c
-	sentry_logs.h
 )
 
 # generic platform / path / symbolizer
diff --git a/src/sentry_logs.c b/src/sentry_logs.c
@@ -145,6 +145,17 @@ skip_precision(const char *fmt_ptr)
     return fmt_ptr;
 }
 
+static const char *
+skip_length(const char *fmt_ptr)
+{
+    while (*fmt_ptr
+        && (*fmt_ptr == 'h' || *fmt_ptr == 'l' || *fmt_ptr == 'L'
+            || *fmt_ptr == 'z' || *fmt_ptr == 'j' || *fmt_ptr == 't')) {
+        fmt_ptr++;
+    }
+    return fmt_ptr;
+}
+
 static void
 populate_message_parameters(
     sentry_value_t attributes, const char *message, va_list args)
@@ -172,13 +183,7 @@ populate_message_parameters(
             fmt_ptr = skip_flags(fmt_ptr);
             fmt_ptr = skip_width(fmt_ptr);
             fmt_ptr = skip_precision(fmt_ptr);
-
-            // Skip length modifiers
-            while (*fmt_ptr
-                && (*fmt_ptr == 'h' || *fmt_ptr == 'l' || *fmt_ptr == 'L'
-                    || *fmt_ptr == 'z' || *fmt_ptr == 'j' || *fmt_ptr == 't')) {
-                fmt_ptr++;
-            }
+            fmt_ptr = skip_length(fmt_ptr);
 
             // Get the conversion specifier
             char conversion = *fmt_ptr;
diff --git a/src/sentry_value.c b/src/sentry_value.c
@@ -317,7 +317,7 @@ sentry_value_new_int32(int32_t value)
 sentry_value_t
 sentry_value_new_double(double value)
 {
-    thing_t *thing = sentry_malloc(sizeof(thing_t));
+    thing_t *thing = SENTRY_MAKE(thing_t);
     if (!thing) {
         return sentry_value_new_null();
     }
@@ -333,7 +333,7 @@ sentry_value_new_double(double value)
 sentry_value_t
 sentry_value_new_int64(int64_t value)
 {
-    thing_t *thing = sentry_malloc(sizeof(thing_t));
+    thing_t *thing = SENTRY_MAKE(thing_t);
     if (!thing) {
         return sentry_value_new_null();
     }
@@ -349,7 +349,7 @@ sentry_value_new_int64(int64_t value)
 sentry_value_t
 sentry_value_new_uint64(uint64_t value)
 {
-    thing_t *thing = sentry_malloc(sizeof(thing_t));
+    thing_t *thing = SENTRY_MAKE(thing_t);
     if (!thing) {
         return sentry_value_new_null();
     }
@@ -666,6 +666,9 @@ sentry__value_as_uuid(sentry_value_t value)
 char *
 sentry__value_stringify(sentry_value_t value)
 {
+    // returns empty string if snprintf fails
+    // (returning -1, so casting this to size_t it becomes > the buffer size)
+    // or if the value is too large for the buffer
 #define STRINGIFY_NUMERIC(fmt, value_fn)                                       \
     do {                                                                       \
         char buf[24];                                                          \
diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt
@@ -28,7 +28,7 @@ add_executable(sentry_test_unit
 	test_fuzzfailures.c
 	test_info.c
 	test_logger.c
-    test_logs.c
+	test_logs.c
 	test_modulefinder.c
 	test_mpack.c
 	test_options.c

Original file line number	Diff line number	Diff line change
`@@ -317,7 +317,7 @@ sentry_value_new_int32(int32_t value)`
`317`	`317`	`sentry_value_t`
`318`	`318`	`sentry_value_new_double(double value)`
`319`	`319`	`{`
`320`		`- thing_t *thing = sentry_malloc(sizeof(thing_t));`
	`320`	`+ thing_t *thing = SENTRY_MAKE(thing_t);`
`321`	`321`	`if (!thing) {`
`322`	`322`	`return sentry_value_new_null();`
`323`	`323`	`}`
`@@ -333,7 +333,7 @@ sentry_value_new_double(double value)`
`333`	`333`	`sentry_value_t`
`334`	`334`	`sentry_value_new_int64(int64_t value)`
`335`	`335`	`{`
`336`		`- thing_t *thing = sentry_malloc(sizeof(thing_t));`
	`336`	`+ thing_t *thing = SENTRY_MAKE(thing_t);`
`337`	`337`	`if (!thing) {`
`338`	`338`	`return sentry_value_new_null();`
`339`	`339`	`}`
`@@ -349,7 +349,7 @@ sentry_value_new_int64(int64_t value)`
`349`	`349`	`sentry_value_t`
`350`	`350`	`sentry_value_new_uint64(uint64_t value)`
`351`	`351`	`{`
`352`		`- thing_t *thing = sentry_malloc(sizeof(thing_t));`
	`352`	`+ thing_t *thing = SENTRY_MAKE(thing_t);`
`353`	`353`	`if (!thing) {`
`354`	`354`	`return sentry_value_new_null();`
`355`	`355`	`}`
`@@ -666,6 +666,9 @@ sentry__value_as_uuid(sentry_value_t value)`
`666`	`666`	`char *`
`667`	`667`	`sentry__value_stringify(sentry_value_t value)`
`668`	`668`	`{`
	`669`	`+ // returns empty string if snprintf fails`
	`670`	`+ // (returning -1, so casting this to size_t it becomes > the buffer size)`
	`671`	`+ // or if the value is too large for the buffer`
`669`	`672`	`#define STRINGIFY_NUMERIC(fmt, value_fn) \`
`670`	`673`	`do { \`
`671`	`674`	`char buf[24]; \`