From 2ab2ce57e665d77932348b6489836e5c31e9896b Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 8 May 2023 19:46:31 -0500 Subject: gtestutils: Improve g_assert_cmpuint While x86_64 has enough precision in long double to do a round trip from guint64 to long double and back, this is platform-specific, and is a disservice to users trying to debug failing unit tests on other architectures where it loses precision for g_assert_cmp{int,uint,hex}. See also https://bugzilla.gnome.org/show_bug.cgi?id=788385 which mentions having to add casts to specifically silence the compiler on platforms where the precision loss occurs. Meanwhile, g_assert_cmpuint() does an unsigned comparison, but outputs signed values if the comparison fails, which is confusing. Fix both issues by introducing a new g_assertion_message_cmpint() function with a new 'u' numtype. For backwards compatibility, the macros still call into the older g_assertion_message_cmpnum() when not targetting 2.78, and that function still works when passed 'i' and 'x' types even though code compiled for 2.78 and later will never invoke it with numtype anything other than 'f'. Note that g_assert_cmpmem can also take advantage of the new code, even though in practice, comparison between two size_t values representing array lengths that can actually be compiled is unlikely to have ever hit the precision loss. The macros in signals.c test code does not have to worry about versioning, since it is not part of the glib library proper. Closes #2997 Signed-off-by: Eric Blake --- docs/reference/glib/glib-sections.txt.in | 1 + glib/gtestutils.c | 29 ++++++++++++++++-- glib/gtestutils.h | 51 ++++++++++++++++++++++++++++++++ gobject/tests/signals.c | 4 +-- 4 files changed, 81 insertions(+), 4 deletions(-) diff --git a/docs/reference/glib/glib-sections.txt.in b/docs/reference/glib/glib-sections.txt.in index a674f8320..012aeef77 100644 --- a/docs/reference/glib/glib-sections.txt.in +++ b/docs/reference/glib/glib-sections.txt.in @@ -3401,6 +3401,7 @@ g_test_trap_assertions g_assertion_message g_assertion_message_expr g_assertion_message_cmpstr +g_assertion_message_cmpint g_assertion_message_cmpnum g_assertion_message_error g_test_assert_expected_messages_internal diff --git a/glib/gtestutils.c b/glib/gtestutils.c index d5cf23683..0164804a9 100644 --- a/glib/gtestutils.c +++ b/glib/gtestutils.c @@ -3486,6 +3486,29 @@ g_assertion_message_expr (const char *domain, g_abort (); } +void +g_assertion_message_cmpint (const char *domain, + const char *file, + int line, + const char *func, + const char *expr, + guint64 arg1, + const char *cmp, + guint64 arg2, + char numtype) +{ + char *s = NULL; + + switch (numtype) + { + case 'i': s = g_strdup_printf ("assertion failed (%s): (%" G_GINT64_MODIFIER "i %s %" G_GINT64_MODIFIER "i)", expr, (gint64) arg1, cmp, (gint64) arg2); break; + case 'u': s = g_strdup_printf ("assertion failed (%s): (%" G_GINT64_MODIFIER "u %s %" G_GINT64_MODIFIER "u)", expr, arg1, cmp, arg2); break; + case 'x': s = g_strdup_printf ("assertion failed (%s): (0x%08" G_GINT64_MODIFIER "x %s 0x%08" G_GINT64_MODIFIER "x)", expr, arg1, cmp, arg2); break; + } + g_assertion_message (domain, file, line, func, s); + g_free (s); +} + void g_assertion_message_cmpnum (const char *domain, const char *file, @@ -3501,8 +3524,10 @@ g_assertion_message_cmpnum (const char *domain, switch (numtype) { - case 'i': s = g_strdup_printf ("assertion failed (%s): (%" G_GINT64_MODIFIER "i %s %" G_GINT64_MODIFIER "i)", expr, (gint64) arg1, cmp, (gint64) arg2); break; - case 'x': s = g_strdup_printf ("assertion failed (%s): (0x%08" G_GINT64_MODIFIER "x %s 0x%08" G_GINT64_MODIFIER "x)", expr, (guint64) arg1, cmp, (guint64) arg2); break; + case 'i': + case 'x': + /* Backwards compatibility to apps compiled before 2.78 */ + g_assertion_message_cmpint (domain, file, line, func, expr, (guint64) arg1, cmp, (guint64) arg2, numtype); break; case 'f': s = g_strdup_printf ("assertion failed (%s): (%.9g %s %.9g)", expr, (double) arg1, cmp, (double) arg2); break; /* ideally use: floats=%.7g double=%.17g */ } diff --git a/glib/gtestutils.h b/glib/gtestutils.h index 86ee4e521..9406ff0d8 100644 --- a/glib/gtestutils.h +++ b/glib/gtestutils.h @@ -49,6 +49,26 @@ typedef void (*GTestFixtureFunc) (gpointer fixture, g_assertion_message_cmpstr (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ #s1 " " #cmp " " #s2, __s1, #cmp, __s2); \ } G_STMT_END +#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_78 +#define g_assert_cmpint(n1, cmp, n2) G_STMT_START { \ + gint64 __n1 = (n1), __n2 = (n2); \ + if (__n1 cmp __n2) ; else \ + g_assertion_message_cmpint (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ + #n1 " " #cmp " " #n2, __n1, #cmp, __n2, 'i'); \ + } G_STMT_END +#define g_assert_cmpuint(n1, cmp, n2) G_STMT_START { \ + guint64 __n1 = (n1), __n2 = (n2); \ + if (__n1 cmp __n2) ; else \ + g_assertion_message_cmpint (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ + #n1 " " #cmp " " #n2, __n1, #cmp, __n2, 'u'); \ + } G_STMT_END +#define g_assert_cmphex(n1, cmp, n2) G_STMT_START { \ + guint64 __n1 = (n1), __n2 = (n2); \ + if (__n1 cmp __n2) ; else \ + g_assertion_message_cmpint (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ + #n1 " " #cmp " " #n2, __n1, #cmp, __n2, 'x'); \ + } G_STMT_END +#else /* GLIB_VERSION_MIN_REQUIRED < GLIB_VERSION_2_78 */ #define g_assert_cmpint(n1, cmp, n2) G_STMT_START { \ gint64 __n1 = (n1), __n2 = (n2); \ if (__n1 cmp __n2) ; else \ @@ -67,6 +87,7 @@ typedef void (*GTestFixtureFunc) (gpointer fixture, g_assertion_message_cmpnum (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ #n1 " " #cmp " " #n2, (long double) __n1, #cmp, (long double) __n2, 'x'); \ } G_STMT_END +#endif /* GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_78 */ #define g_assert_cmpfloat(n1,cmp,n2) G_STMT_START { \ long double __n1 = (long double) (n1), __n2 = (long double) (n2); \ if (__n1 cmp __n2) ; else \ @@ -80,6 +101,25 @@ typedef void (*GTestFixtureFunc) (gpointer fixture, g_assertion_message_cmpnum (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ #n1 " == " #n2 " (+/- " #epsilon ")", __n1, "==", __n2, 'f'); \ } G_STMT_END +#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_78 +#define g_assert_cmpmem(m1, l1, m2, l2) G_STMT_START {\ + gconstpointer __m1 = m1, __m2 = m2; \ + size_t __l1 = (size_t) l1, __l2 = (size_t) l2; \ + if (__l1 != 0 && __m1 == NULL) \ + g_assertion_message (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ + "assertion failed (" #l1 " == 0 || " #m1 " != NULL)"); \ + else if (__l2 != 0 && __m2 == NULL) \ + g_assertion_message (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ + "assertion failed (" #l2 " == 0 || " #m2 " != NULL)"); \ + else if (__l1 != __l2) \ + g_assertion_message_cmpint (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ + #l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", \ + __l1, "==", __l2, 'u'); \ + else if (__l1 != 0 && __m2 != NULL && memcmp (__m1, __m2, __l1) != 0) \ + g_assertion_message (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ + "assertion failed (" #m1 " == " #m2 ")"); \ + } G_STMT_END +#else /* GLIB_VERSION_MIN_REQUIRED < GLIB_VERSION_2_78 */ #define g_assert_cmpmem(m1, l1, m2, l2) G_STMT_START {\ gconstpointer __m1 = m1, __m2 = m2; \ size_t __l1 = (size_t) l1, __l2 = (size_t) l2; \ @@ -97,6 +137,7 @@ typedef void (*GTestFixtureFunc) (gpointer fixture, g_assertion_message (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ "assertion failed (" #m1 " == " #m2 ")"); \ } G_STMT_END +#endif /* GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_78 */ #define g_assert_cmpvariant(v1, v2) \ G_STMT_START \ { \ @@ -567,6 +608,16 @@ void g_assertion_message_cmpstrv (const char *domain, const char * const *arg1, const char * const *arg2, gsize first_wrong_idx) G_ANALYZER_NORETURN; +GLIB_AVAILABLE_IN_2_78 +void g_assertion_message_cmpint (const char *domain, + const char *file, + int line, + const char *func, + const char *expr, + guint64 arg1, + const char *cmp, + guint64 arg2, + char numtype) G_ANALYZER_NORETURN; GLIB_AVAILABLE_IN_ALL void g_assertion_message_cmpnum (const char *domain, const char *file, diff --git a/gobject/tests/signals.c b/gobject/tests/signals.c index ef52255cd..ec9a584ca 100644 --- a/gobject/tests/signals.c +++ b/gobject/tests/signals.c @@ -4,13 +4,13 @@ #define g_assert_cmpflags(type,n1, cmp, n2) G_STMT_START { \ type __n1 = (n1), __n2 = (n2); \ if (__n1 cmp __n2) ; else \ - g_assertion_message_cmpnum (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ + g_assertion_message_cmpint (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ #n1 " " #cmp " " #n2, __n1, #cmp, __n2, 'i'); \ } G_STMT_END #define g_assert_cmpenum(type,n1, cmp, n2) G_STMT_START { \ type __n1 = (n1), __n2 = (n2); \ if (__n1 cmp __n2) ; else \ - g_assertion_message_cmpnum (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ + g_assertion_message_cmpint (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ #n1 " " #cmp " " #n2, __n1, #cmp, __n2, 'i'); \ } G_STMT_END -- cgit v1.2.1 From 171bcd524aa0ca0587bd55b26ac4e709fb4bd5cf Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 9 May 2023 08:42:37 -0500 Subject: gtestutils: Style touchup Use more modern styling to the code added in the previous patch: - split 'label: stmt; stmt;' into multiple lines - add default: label with g_assert_not_reached() [yes, it's a bit weird adding an assertion inside code that handles assertions, but we should be okay since g_assertion_message_* are not public functions and should only be used by our macros] - use for shorter format strings Note, however, that using uint64_t in gtestutils.h is not feasible, since it would require adding an '#include ' with potential unintended namespace pollution to older clients. Signed-off-by: Eric Blake --- glib/gtestutils.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/glib/gtestutils.c b/glib/gtestutils.c index 0164804a9..0f0853259 100644 --- a/glib/gtestutils.c +++ b/glib/gtestutils.c @@ -33,6 +33,7 @@ #include #include #include +#include #ifdef HAVE_SYS_RESOURCE_H #include #endif @@ -3501,9 +3502,23 @@ g_assertion_message_cmpint (const char *domain, switch (numtype) { - case 'i': s = g_strdup_printf ("assertion failed (%s): (%" G_GINT64_MODIFIER "i %s %" G_GINT64_MODIFIER "i)", expr, (gint64) arg1, cmp, (gint64) arg2); break; - case 'u': s = g_strdup_printf ("assertion failed (%s): (%" G_GINT64_MODIFIER "u %s %" G_GINT64_MODIFIER "u)", expr, arg1, cmp, arg2); break; - case 'x': s = g_strdup_printf ("assertion failed (%s): (0x%08" G_GINT64_MODIFIER "x %s 0x%08" G_GINT64_MODIFIER "x)", expr, arg1, cmp, arg2); break; + case 'i': + s = g_strdup_printf ("assertion failed (%s): " + "(%" PRIi64 " %s %" PRIi64 ")", + expr, (int64_t) arg1, cmp, (int64_t) arg2); + break; + case 'u': + s = g_strdup_printf ("assertion failed (%s): " + "(%" PRIu64 " %s %" PRIu64 ")", + expr, (uint64_t) arg1, cmp, (uint64_t) arg2); + break; + case 'x': + s = g_strdup_printf ("assertion failed (%s): " + "(0x%08" PRIx64 " %s 0x%08" PRIx64 ")", + expr, (uint64_t) arg1, cmp, (uint64_t) arg2); + break; + default: + g_assert_not_reached (); } g_assertion_message (domain, file, line, func, s); g_free (s); @@ -3524,12 +3539,16 @@ g_assertion_message_cmpnum (const char *domain, switch (numtype) { + case 'f': s = g_strdup_printf ("assertion failed (%s): (%.9g %s %.9g)", expr, (double) arg1, cmp, (double) arg2); break; + /* ideally use: floats=%.7g double=%.17g */ case 'i': case 'x': /* Backwards compatibility to apps compiled before 2.78 */ - g_assertion_message_cmpint (domain, file, line, func, expr, (guint64) arg1, cmp, (guint64) arg2, numtype); break; - case 'f': s = g_strdup_printf ("assertion failed (%s): (%.9g %s %.9g)", expr, (double) arg1, cmp, (double) arg2); break; - /* ideally use: floats=%.7g double=%.17g */ + g_assertion_message_cmpint (domain, file, line, func, expr, + (guint64) arg1, cmp, (guint64) arg2, numtype); + break; + default: + g_assert_not_reached (); } g_assertion_message (domain, file, line, func, s); g_free (s); -- cgit v1.2.1