From 40d9892fd0a391ae8b56fc1fcf56f885841a7ef5 Mon Sep 17 00:00:00 2001 From: Roman Savchenko Date: Thu, 7 Dec 2023 17:43:56 +0100 Subject: [PATCH] Remove options memory leak during consent setting --- CHANGELOG.md | 1 + src/sentry_core.c | 40 +++++++++++++++++++-------------------- tests/unit/test_consent.c | 3 +++ 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0550d4724..88daef74d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Updated `crashpad` to 2023-11-24. ([#912](https://github.com/getsentry/sentry-native/pull/912), [crashpad#91](https://github.com/getsentry/crashpad/pull/91)) - Fixing `crashpad` build for Windows on ARM64. ([#919](https://github.com/getsentry/sentry-native/pull/919), [crashpad#90](https://github.com/getsentry/crashpad/pull/90), [crashpad#92](https://github.com/getsentry/crashpad/pull/92), [crashpad#93](https://github.com/getsentry/crashpad/pull/93), [crashpad#94](https://github.com/getsentry/crashpad/pull/94)) +- Remove options memory leak during consent setting. ([#922](https://github.com/getsentry/sentry-native/pull/922)) **Thank you**: diff --git a/src/sentry_core.c b/src/sentry_core.c index 43309b67f..24cf52539 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -292,29 +292,27 @@ set_user_consent(sentry_user_consent_t new_val) { SENTRY_WITH_OPTIONS (options) { if (sentry__atomic_store((long *)&options->user_consent, new_val) - == new_val) { - // nothing was changed - break; // SENTRY_WITH_OPTIONS - } - - if (options->backend && options->backend->user_consent_changed_func) { - options->backend->user_consent_changed_func(options->backend); - } + != new_val) { + if (options->backend + && options->backend->user_consent_changed_func) { + options->backend->user_consent_changed_func(options->backend); + } - sentry_path_t *consent_path - = sentry__path_join_str(options->database_path, "user-consent"); - switch (new_val) { - case SENTRY_USER_CONSENT_GIVEN: - sentry__path_write_buffer(consent_path, "1\n", 2); - break; - case SENTRY_USER_CONSENT_REVOKED: - sentry__path_write_buffer(consent_path, "0\n", 2); - break; - case SENTRY_USER_CONSENT_UNKNOWN: - sentry__path_remove(consent_path); - break; + sentry_path_t *consent_path + = sentry__path_join_str(options->database_path, "user-consent"); + switch (new_val) { + case SENTRY_USER_CONSENT_GIVEN: + sentry__path_write_buffer(consent_path, "1\n", 2); + break; + case SENTRY_USER_CONSENT_REVOKED: + sentry__path_write_buffer(consent_path, "0\n", 2); + break; + case SENTRY_USER_CONSENT_UNKNOWN: + sentry__path_remove(consent_path); + break; + } + sentry__path_free(consent_path); } - sentry__path_free(consent_path); } } diff --git a/tests/unit/test_consent.c b/tests/unit/test_consent.c index f26af345b..4196cb317 100644 --- a/tests/unit/test_consent.c +++ b/tests/unit/test_consent.c @@ -28,6 +28,9 @@ SENTRY_TEST(basic_consent_tracking) init_consenting_sentry(); sentry_user_consent_give(); + // testing correct options ref/decref during double `sentry_user_consent_give` call + // see https://github.com/getsentry/sentry-native/pull/922 + sentry_user_consent_give(); TEST_CHECK_INT_EQUAL(sentry_user_consent_get(), SENTRY_USER_CONSENT_GIVEN); sentry_close(); init_consenting_sentry();