Hi,
On Tue, Jan 19, 2016 at 05:55:22PM +0100, Christophe Fergeau wrote:
> 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
Sure
Acked-by: Victor Toso <victortoso@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel