From 517a22dcf6dcb9226fdfc805dedcca3febf06cf5 Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Sun, 27 Dec 2020 13:47:36 +0100 Subject: Rationalize NULL/empty string handling in media_art_strip_invalid_entities() We return NULL if input is NULL and a newly allocated empty string if input is "". Some comments disagreed with this, and it's possible the change causes a memory leak in some app, but the alternative of returning NULL when passed "" is dangerous as some code may free the return value in this case. (In fact, libmediaart itself does so). Also, make behaviour occur independently of whether `G_ENABLE_CONSISTENCY_CHECKS` was defined at build time. --- libmediaart/cache.c | 17 ++++++++++++++--- tests/mediaarttest.c | 18 ++++++++++-------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/libmediaart/cache.c b/libmediaart/cache.c index ecbc7a1..b5b023b 100644 --- a/libmediaart/cache.c +++ b/libmediaart/cache.c @@ -101,7 +101,7 @@ media_art_strip_find_next_block (const gchar *original, /** * media_art_strip_invalid_entities: - * @original: original string + * @original: (nullable): original string * * Strip a albumname or artistname string to prepare it for calculating the * media art path with it. Certain characters and charactersets will be stripped @@ -116,7 +116,7 @@ media_art_strip_find_next_block (const gchar *original, * 3. Multiples of space characters are removed. * * Returns: @original stripped of invalid characters which must be - * freed. On error or if @original is empty, %NULL is returned. + * freed. On error or if @original is NULL, %NULL is returned. * * Since: 0.2.0 */ @@ -140,7 +140,8 @@ media_art_strip_invalid_entities (const gchar *original) { 0, 0 } }; - g_return_val_if_fail (original != NULL, NULL); + if (original == NULL) + return NULL; str_no_blocks = g_string_new (""); @@ -284,6 +285,10 @@ media_art_get_file (const gchar *artist, /* http://live.gnome.org/MediaArtStorageSpec */ + g_return_val_if_fail (!artist || g_utf8_validate (artist, -1, NULL), FALSE); + g_return_val_if_fail (!title || g_utf8_validate (title, -1, NULL), FALSE); + g_return_val_if_fail (!prefix || g_utf8_validate (prefix, -1, NULL), FALSE); + if (cache_file) { *cache_file = NULL; } @@ -381,6 +386,10 @@ media_art_get_path (const gchar *artist, { GFile *cache_file = NULL; + g_return_val_if_fail (!artist || g_utf8_validate (artist, -1, NULL), FALSE); + g_return_val_if_fail (!title || g_utf8_validate (title, -1, NULL), FALSE); + g_return_val_if_fail (!prefix || g_utf8_validate (prefix, -1, NULL), FALSE); + /* Rules: * 1. artist OR title must be non-NULL. * 2. cache_file must be non-NULL @@ -424,6 +433,8 @@ media_art_remove (const gchar *artist, gboolean success = TRUE; g_return_val_if_fail (artist != NULL && artist[0] != '\0', FALSE); + g_return_val_if_fail (g_utf8_validate (artist, -1, NULL), FALSE); + g_return_val_if_fail (!album || g_utf8_validate (album, -1, NULL), FALSE); dirname = g_build_filename (g_get_user_cache_dir (), "media-art", NULL); diff --git a/tests/mediaarttest.c b/tests/mediaarttest.c index cef36c2..93ac684 100644 --- a/tests/mediaarttest.c +++ b/tests/mediaarttest.c @@ -112,18 +112,20 @@ test_mediaart_stripping_failures_subprocess (void) static void test_mediaart_stripping_failures (void) { - gchar *stripped = NULL; + gchar *stripped, *input = NULL; /* a. Return NULL for NULL (subprocess) - * b. Return NULL for "" + * b. Return a copy for "" */ - stripped = media_art_strip_invalid_entities (""); - g_assert (stripped); - g_assert_cmpstr (stripped, ==, ""); + stripped = media_art_strip_invalid_entities (NULL); + g_assert (!stripped); - g_test_trap_subprocess ("/mediaart/stripping_failures/subprocess", 0, 0); - g_test_trap_assert_failed (); - g_test_trap_assert_stderr ("*assertion 'original != NULL' failed*"); + input = ""; + stripped = media_art_strip_invalid_entities (input); + g_assert (stripped); + g_assert (stripped != input); + g_assert (strcmp(stripped, "") == 0); + g_free (stripped); } -- cgit v1.2.1