From e2cd0962c27eb018555783f8ce65130bd399389c Mon Sep 17 00:00:00 2001
From: Philip Withnall <pwithnall@gnome.org>
Date: Mon, 27 Nov 2023 11:00:37 +0000
Subject: [PATCH] gmessages: Document error case for
 g_printf_string_upper_bound()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Spotted in https://gitlab.gnome.org/GNOME/glib/-/issues/3187.

In an ideal world, this API would have been designed with an error
return path to begin with — perhaps by returning a `GError` or a
`gssize`. We can’t change the API now, though, which leads to this
slightly awkward “0 indicates an error or success” pattern.

I think that’s justified in this case because:
 - This API does not see much use.
 - Format strings tend to be literals, and almost always are
   non-zero-length, so it tends to be statically possible to determine
   that the function won’t return zero on success.
 - If callers do need to differentiate the two zero return value cases,
   they can just call `g_vsnprintf()` directly instead.

Signed-off-by: Philip Withnall <pwithnall@gnome.org>

Helps: #3187
---
 glib/gmessages.c         | 17 +++++++++++++++--
 glib/tests/test-printf.c |  3 +++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/glib/gmessages.c b/glib/gmessages.c
index cad2faadf..a47e347a3 100644
--- a/glib/gmessages.c
+++ b/glib/gmessages.c
@@ -3391,12 +3391,25 @@ g_printerr (const gchar *format,
  * Calculates the maximum space needed to store the output
  * of the sprintf() function.
  *
- * Returns: the maximum space needed to store the formatted string
+ * If @format or @args are invalid, `0` is returned. This could happen if, for
+ * example, @format contains an `%lc` or `%ls` placeholder and @args contains a
+ * wide character which cannot be represented in multibyte encoding. `0`
+ * can also be returned legitimately if, for example, @format is `%s` and @args
+ * is an empty string. The caller is responsible for differentiating these two
+ * return cases if necessary. It is recommended to not use `%lc` or `%ls`
+ * placeholders in any case, as their behaviour is locale-dependent.
+ *
+ * Returns: the maximum space needed to store the formatted string, or `0` on error
  */
 gsize
 g_printf_string_upper_bound (const gchar *format,
                              va_list      args)
 {
   gchar c;
-  return _g_vsnprintf (&c, 1, format, args) + 1;
+  int count = _g_vsnprintf (&c, 1, format, args);
+
+  if (count < 0)
+    return 0;
+
+  return count + 1;
 }
diff --git a/glib/tests/test-printf.c b/glib/tests/test-printf.c
index 261b03812..faec8e110 100644
--- a/glib/tests/test-printf.c
+++ b/glib/tests/test-printf.c
@@ -897,6 +897,9 @@ test_upper_bound (void)
 
   res = upper_bound ("bla %s %d: %g\n", "bla", 123, 0.123);
   g_assert_cmpint (res, ==, 20);
+
+  res = upper_bound ("Invalid case: %ls", L"\xD800" /* incomplete surrogate pair */);
+  g_assert_cmpint (res, ==, 0);
 }
 
 #if !defined(__APPLE__) && !defined(__FreeBSD__)