From f4fdb66d0399e3223c068c663e7621ea19b71a7b Mon Sep 17 00:00:00 2001 From: psykose Date: Mon, 12 Feb 2024 14:59:42 +0000 Subject: [PATCH] use g_autoptr again but with a custom CLEANUP_FUNC my prior analysis was completely wrong, and the original crash was seen because sqlite3_malloc pointers passed to g_free cause a mismatched malloc/free, not because of lack of initialisation. use autoptr with an overriden CLEANUP_FUNC instead. for cd_mapping_db_load/cd_device_db_remove, keep using manual sqlite3_free since we reuse the same error_msg multiple times. --- src/cd-common.h | 3 +++ src/cd-device-db.c | 21 +++++++-------------- src/cd-mapping-db.c | 24 ++++++++---------------- src/cd-profile-db.c | 15 +++++---------- 4 files changed, 23 insertions(+), 40 deletions(-) diff --git a/src/cd-common.h b/src/cd-common.h index adf93438..c60061ae 100644 --- a/src/cd-common.h +++ b/src/cd-common.h @@ -26,6 +26,7 @@ #include #include +#include #define COLORD_DBUS_SERVICE "org.freedesktop.ColorManager" #define COLORD_DBUS_PATH "/org/freedesktop/ColorManager" @@ -37,6 +38,8 @@ #define CD_DBUS_METADATA_KEY_LEN_MAX 256 /* chars */ #define CD_DBUS_METADATA_VALUE_LEN_MAX 4096 /* chars */ +typedef char sqlite_str; +G_DEFINE_AUTOPTR_CLEANUP_FUNC (sqlite_str, sqlite3_free) #define CD_CLIENT_ERROR cd_client_error_quark() diff --git a/src/cd-device-db.c b/src/cd-device-db.c index ac87a527..e97c8823 100644 --- a/src/cd-device-db.c +++ b/src/cd-device-db.c @@ -48,7 +48,7 @@ cd_device_db_load (CdDeviceDb *ddb, { CdDeviceDbPrivate *priv = GET_PRIVATE (ddb); const gchar *statement; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gint rc; g_autofree gchar *path = NULL; @@ -82,7 +82,6 @@ cd_device_db_load (CdDeviceDb *ddb, NULL, NULL, &error_msg); if (rc != SQLITE_OK) { g_debug ("CdDeviceDb: creating table to repair: %s", error_msg); - sqlite3_free (error_msg); statement = "CREATE TABLE devices (" "device_id TEXT PRIMARY KEY," "device TEXT);"; @@ -109,7 +108,7 @@ cd_device_db_empty (CdDeviceDb *ddb, { CdDeviceDbPrivate *priv = GET_PRIVATE (ddb); const gchar *statement; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gint rc; g_return_val_if_fail (CD_IS_DEVICE_DB (ddb), FALSE); @@ -124,7 +123,6 @@ cd_device_db_empty (CdDeviceDb *ddb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); return FALSE; } return TRUE; @@ -137,7 +135,7 @@ cd_device_db_add (CdDeviceDb *ddb, { CdDeviceDbPrivate *priv = GET_PRIVATE (ddb); gboolean ret = TRUE; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; @@ -157,7 +155,6 @@ cd_device_db_add (CdDeviceDb *ddb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); ret = FALSE; goto out; } @@ -175,7 +172,7 @@ cd_device_db_set_property (CdDeviceDb *ddb, { CdDeviceDbPrivate *priv = GET_PRIVATE (ddb); gboolean ret = TRUE; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; @@ -196,7 +193,6 @@ cd_device_db_set_property (CdDeviceDb *ddb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); ret = FALSE; goto out; } @@ -277,7 +273,7 @@ cd_device_db_get_property (CdDeviceDb *ddb, GError **error) { CdDeviceDbPrivate *priv = GET_PRIVATE (ddb); - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; gchar *value = NULL; @@ -305,7 +301,6 @@ cd_device_db_get_property (CdDeviceDb *ddb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); goto out; } @@ -331,7 +326,7 @@ cd_device_db_get_devices (CdDeviceDb *ddb, GError **error) { CdDeviceDbPrivate *priv = GET_PRIVATE (ddb); - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; GPtrArray *array = NULL; @@ -355,7 +350,6 @@ cd_device_db_get_devices (CdDeviceDb *ddb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); goto out; } @@ -372,7 +366,7 @@ cd_device_db_get_properties (CdDeviceDb *ddb, GError **error) { CdDeviceDbPrivate *priv = GET_PRIVATE (ddb); - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; GPtrArray *array = NULL; @@ -398,7 +392,6 @@ cd_device_db_get_properties (CdDeviceDb *ddb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); goto out; } diff --git a/src/cd-mapping-db.c b/src/cd-mapping-db.c index 291274a4..edf944c1 100644 --- a/src/cd-mapping-db.c +++ b/src/cd-mapping-db.c @@ -67,7 +67,7 @@ cd_mapping_db_open (CdMappingDb *mdb, GError **error) { CdMappingDbPrivate *priv = GET_PRIVATE (mdb); - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gint rc; g_autofree gchar *path = NULL; @@ -97,7 +97,6 @@ cd_mapping_db_open (CdMappingDb *mdb, if (rc != SQLITE_OK) { /* Database appears to be mangled, so wipe it and try again */ sqlite3_close (priv->db); - sqlite3_free(error_msg); priv->db = NULL; if (retry) { @@ -230,7 +229,7 @@ cd_mapping_db_empty (CdMappingDb *mdb, { CdMappingDbPrivate *priv = GET_PRIVATE (mdb); const gchar *statement; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gint rc; g_return_val_if_fail (CD_IS_MAPPING_DB (mdb), FALSE); @@ -245,7 +244,6 @@ cd_mapping_db_empty (CdMappingDb *mdb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); return FALSE; } return TRUE; @@ -259,7 +257,7 @@ cd_mapping_db_add (CdMappingDb *mdb, { CdMappingDbPrivate *priv = GET_PRIVATE (mdb); gboolean ret = TRUE; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; gint64 timestamp; @@ -282,7 +280,6 @@ cd_mapping_db_add (CdMappingDb *mdb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); ret = FALSE; goto out; } @@ -307,7 +304,7 @@ cd_mapping_db_clear_timestamp (CdMappingDb *mdb, { CdMappingDbPrivate *priv = GET_PRIVATE (mdb); gboolean ret = TRUE; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; @@ -328,7 +325,6 @@ cd_mapping_db_clear_timestamp (CdMappingDb *mdb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); ret = FALSE; goto out; } @@ -351,7 +347,7 @@ cd_mapping_db_remove (CdMappingDb *mdb, { CdMappingDbPrivate *priv = GET_PRIVATE (mdb); gboolean ret = TRUE; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; @@ -371,7 +367,6 @@ cd_mapping_db_remove (CdMappingDb *mdb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); ret = FALSE; goto out; } @@ -406,7 +401,7 @@ cd_mapping_db_get_profiles (CdMappingDb *mdb, GError **error) { CdMappingDbPrivate *priv = GET_PRIVATE (mdb); - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; GPtrArray *array = NULL; @@ -433,7 +428,6 @@ cd_mapping_db_get_profiles (CdMappingDb *mdb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); goto out; } @@ -456,7 +450,7 @@ cd_mapping_db_get_devices (CdMappingDb *mdb, GError **error) { CdMappingDbPrivate *priv = GET_PRIVATE (mdb); - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; GPtrArray *array = NULL; @@ -483,7 +477,6 @@ cd_mapping_db_get_devices (CdMappingDb *mdb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); goto out; } @@ -522,7 +515,7 @@ cd_mapping_db_get_timestamp (CdMappingDb *mdb, GError **error) { CdMappingDbPrivate *priv = GET_PRIVATE (mdb); - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; guint64 timestamp = G_MAXUINT64; @@ -548,7 +541,6 @@ cd_mapping_db_get_timestamp (CdMappingDb *mdb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); goto out; } diff --git a/src/cd-profile-db.c b/src/cd-profile-db.c index 124fa09f..7d6b09a6 100644 --- a/src/cd-profile-db.c +++ b/src/cd-profile-db.c @@ -48,7 +48,7 @@ cd_profile_db_load (CdProfileDb *pdb, { CdProfileDbPrivate *priv = GET_PRIVATE (pdb); const gchar *statement; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gint rc; g_autofree gchar *path = NULL; @@ -69,7 +69,6 @@ cd_profile_db_load (CdProfileDb *pdb, CD_CLIENT_ERROR_INTERNAL, "Can't open database: %s\n", sqlite3_errmsg (priv->db)); - sqlite3_free (error_msg); sqlite3_close (priv->db); return FALSE; } @@ -98,7 +97,7 @@ cd_profile_db_empty (CdProfileDb *pdb, GError **error) { CdProfileDbPrivate *priv = GET_PRIVATE (pdb); const gchar *statement; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gint rc; g_return_val_if_fail (CD_IS_PROFILE_DB (pdb), FALSE); @@ -113,7 +112,6 @@ cd_profile_db_empty (CdProfileDb *pdb, GError **error) CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); return FALSE; } return TRUE; @@ -129,7 +127,7 @@ cd_profile_db_set_property (CdProfileDb *pdb, { CdProfileDbPrivate *priv = GET_PRIVATE (pdb); gboolean ret = TRUE; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; @@ -151,7 +149,6 @@ cd_profile_db_set_property (CdProfileDb *pdb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); ret = FALSE; goto out; } @@ -169,7 +166,7 @@ cd_profile_db_remove (CdProfileDb *pdb, { CdProfileDbPrivate *priv = GET_PRIVATE (pdb); gboolean ret = TRUE; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement = NULL; gint rc; @@ -190,7 +187,6 @@ cd_profile_db_remove (CdProfileDb *pdb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); ret = FALSE; goto out; } @@ -223,7 +219,7 @@ cd_profile_db_get_property (CdProfileDb *pdb, { CdProfileDbPrivate *priv = GET_PRIVATE (pdb); gboolean ret = TRUE; - char *error_msg = NULL; + g_autoptr(sqlite_str) error_msg = NULL; gchar *statement; gint rc; @@ -250,7 +246,6 @@ cd_profile_db_get_property (CdProfileDb *pdb, CD_CLIENT_ERROR_INTERNAL, "SQL error: %s", error_msg); - sqlite3_free (error_msg); goto out; } out: