[Spice-devel] [spice-common v3 3/7] log: Add test case for logging code

[Christophe Fergeau]

On Fri, Dec 18, 2015 at 11:40:51AM +0100, Christophe Fergeau wrote:
> On Thu, Dec 17, 2015 at 10:43:05PM +0100, Victor Toso wrote:
> > > +/* Checks that spice_return_if_fail() aborts by default */
> > > +static void test_spice_fatal_return_if_fail(void)
> > > +{
> > > +    if (g_test_subprocess()) {
> > > +        spice_return_if_fail(FALSE);
> > > +        return;
> > > +    }
> > > +    g_test_trap_subprocess(NULL, 0, 0);
> > > +    g_test_trap_assert_failed();
> > > +}
> > > +
> > > +/* Checks that g_return_if_fail() does not abort by default */
> > > +static void test_spice_non_fatal_g_return_if_fail(void)
> > > +{
> > > +    char *pattern = g_strconcat ("*", G_STRFUNC, "*", NULL);
> > > +    g_test_log_set_fatal_handler(non_fatal_warnings_criticals, pattern);
> > > +
> > > +    if (g_test_subprocess()) {
> > > +        g_return_if_fail(FALSE);
> > 
> > pattern leaks here
> 
> I know, but I don't think I can easily free it unless I move the
> g_return_if_fail() to a separate helper function. If
> g_test_log_set_fatal_handler() is not called, then g_return_if_fail()
> will abort. I did not manage to find some preprocessor magic to build
> a compile-time string (rather than an allocated one) with the content I
> want. I can add a comment making it explicit that the leak is on
> purpose. The g_return_if_fail() runs in a subprocess which will end
> right after that call, so in my opinion this leak is acceptable.

An alternative is to add this patch on top of this one:

diff --git a/tests/test-logging.c b/tests/test-logging.c
index e56172a..4635ffa 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -163,8 +163,8 @@ static void test_spice_fatal_return_if_fail(void)
 /* Checks that g_return_if_fail() does not abort by default */
 static void test_spice_non_fatal_g_return_if_fail(void)
 {
-    char *pattern = g_strconcat ("*", G_STRFUNC, "*", NULL);
-    g_test_log_set_fatal_handler(non_fatal_warnings_criticals, pattern);
+    g_test_log_set_fatal_handler(non_fatal_warnings_criticals,
+                                 "*test_spice_non_fatal_g_return_if_fail*");

     if (g_test_subprocess()) {
         g_return_if_fail(FALSE);
@@ -172,7 +172,6 @@ static void test_spice_non_fatal_g_return_if_fail(void)
     }
     g_test_trap_subprocess(NULL, 0, 0);
     g_test_trap_assert_passed();
-    g_free(pattern);
 }

 /* Checks that spice_assert_if_fail() aborts by default */

I think I'll go with that.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20160119/9757cb68/attachment.sig>