[waffle] [PATCH 1/3] waffle: add public func waffle_teardown()
Emil Velikov
emil.l.velikov at gmail.com
Wed Jan 21 06:42:07 PST 2015
On 20 January 2015 at 18:28, Chad Versace <chad.versace at intel.com> wrote:
> On 01/15/2015 07:21 AM, Emil Velikov wrote:
>> It will be used to teardown the global state, make valgrind happy and catch
>> (point out) a bug or two in the GL/EGL/etc implementations.
>>
>> See the man page for more details.
>>
>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>> ---
>> include/waffle/waffle.h | 5 ++
>> man/common/author-emil.velikov.xml | 8 +++
>> man/html.cmake | 2 +
>> man/manpages.cmake | 2 +
>> man/waffle_init.3.xml | 3 +-
>> man/waffle_teardown.3.xml | 115 +++++++++++++++++++++++++++++++++++++
>> src/waffle/api/api_priv.h | 6 +-
>> src/waffle/api/waffle_init.c | 21 +++++++
>> src/waffle/waffle.def.in | 1 +
>> 9 files changed, 159 insertions(+), 4 deletions(-)
>> create mode 100644 man/common/author-emil.velikov.xml
>> create mode 100644 man/waffle_teardown.3.xml
>>
>> diff --git a/include/waffle/waffle.h b/include/waffle/waffle.h
>> index e04b23f..6d200c2 100644
>> --- a/include/waffle/waffle.h
>> +++ b/include/waffle/waffle.h
>> @@ -160,6 +160,11 @@ waffle_enum_to_string(int32_t e);
>> bool
>> waffle_init(const int32_t *attrib_list);
>>
>> +#if WAFFLE_API_VERSION >= 0x0106
>> +bool
>> +waffle_teardown(void);
>> +#endif
>> +
>> bool
>> waffle_make_current(struct waffle_display *dpy,
>> struct waffle_window *window,
>> diff --git a/man/common/author-emil.velikov.xml b/man/common/author-emil.velikov.xml
>> new file mode 100644
>> index 0000000..382a947
>> --- /dev/null
>> +++ b/man/common/author-emil.velikov.xml
>> @@ -0,0 +1,8 @@
>> +<?xml version='1.0'?>
>> +
>> +<author>
>> + <firstname>Emil</firstname>
>> + <surname>Velikov</surname>
>> + <email>emil.l.velikov at gmail.com</email>
>> + <contrib>Contributor</contrib>
>> +</author>
>> diff --git a/man/html.cmake b/man/html.cmake
>> index 59e0490..e783373 100644
>> --- a/man/html.cmake
>> +++ b/man/html.cmake
>> @@ -43,6 +43,7 @@ set(html_outputs
>> ${html_out_dir}/waffle_is_extension_in_string.3.html
>> ${html_out_dir}/waffle_make_current.3.html
>> ${html_out_dir}/waffle_native.3.html
>> + ${html_out_dir}/waffle_teardown.3.html
>> ${html_out_dir}/waffle_wayland.3.html
>> ${html_out_dir}/waffle_window.3.html
>> ${html_out_dir}/waffle_x11_egl.3.html
>> @@ -81,6 +82,7 @@ waffle_add_html(3 waffle_init)
>> waffle_add_html(3 waffle_is_extension_in_string)
>> waffle_add_html(3 waffle_make_current)
>> waffle_add_html(3 waffle_native)
>> +waffle_add_html(3 waffle_teardown)
>> waffle_add_html(3 waffle_wayland)
>> waffle_add_html(3 waffle_window)
>> waffle_add_html(3 waffle_x11_egl)
>> diff --git a/man/manpages.cmake b/man/manpages.cmake
>> index b8ededa..b0c20b3 100644
>> --- a/man/manpages.cmake
>> +++ b/man/manpages.cmake
>> @@ -45,6 +45,7 @@ set(man_outputs
>> ${man_out_dir}/man3/waffle_is_extension_in_string.3
>> ${man_out_dir}/man3/waffle_make_current.3
>> ${man_out_dir}/man3/waffle_native.3
>> + ${man_out_dir}/man3/waffle_teardown.3
>> ${man_out_dir}/man3/waffle_wayland.3
>> ${man_out_dir}/man3/waffle_window.3
>> ${man_out_dir}/man3/waffle_x11_egl.3
>> @@ -81,6 +82,7 @@ waffle_add_manpage(3 waffle_init)
>> waffle_add_manpage(3 waffle_is_extension_in_string)
>> waffle_add_manpage(3 waffle_make_current)
>> waffle_add_manpage(3 waffle_native)
>> +waffle_add_manpage(3 waffle_teardown)
>> waffle_add_manpage(3 waffle_wayland)
>> waffle_add_manpage(3 waffle_window)
>> waffle_add_manpage(3 waffle_x11_egl)
>> diff --git a/man/waffle_init.3.xml b/man/waffle_init.3.xml
>> index a22723d..4dd77d5 100644
>> --- a/man/waffle_init.3.xml
>> +++ b/man/waffle_init.3.xml
>> @@ -60,7 +60,8 @@
>> <para>
>> If a call to <function>waffle_init()</function> fails, no global state is initialized and the caller may safely
>> attempt to call <function>waffle_init()</function> again. If waffle has already been initialized by a successful
>> - call to <function>waffle_init()</function>, then calling <function>waffle_init()</function> again emits the error
>> + call to <function>waffle_init()</function> one has to call <function>waffle_teardown()</function> to clear the
>> + global state. Otherwise calling <function>waffle_init()</function> again emits the error
>> <errorcode>WAFFLE_ERROR_ALREADY_INITIALIZED</errorcode>.
>> </para>
>>
>> diff --git a/man/waffle_teardown.3.xml b/man/waffle_teardown.3.xml
>> new file mode 100644
>> index 0000000..69b18e9
>> --- /dev/null
>> +++ b/man/waffle_teardown.3.xml
>> @@ -0,0 +1,115 @@
>> +<?xml version='1.0'?>
>> +<!DOCTYPE refentry PUBLIC "-//OASIS//DTD DocBook XML V4.2//EN"
>> + "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd">
>> +
>> +<!--
>> + Copyright Intel 2015
>> +
>> + This manual page is licensed under the Creative Commons Attribution-ShareAlike 3.0 United States License (CC BY-SA 3.0
>> + US). To view a copy of this license, visit http://creativecommons.org.license/by-sa/3.0/us.
>> +-->
>> +
>> +<refentry
>> + id="waffle_teardown"
>> + xmlns:xi="http://www.w3.org/2001/XInclude">
>> +
>> + <!-- See http://www.docbook.org/tdg/en/html/refentry.html. -->
>> +
>> + <refmeta>
>> + <refentrytitle>waffle_teardown</refentrytitle>
>> + <manvolnum>3</manvolnum>
>> + </refmeta>
>> +
>> + <refnamediv>
>> + <refname>waffle_teardown</refname>
>> + <refpurpose>Teardown waffle's per-process global state</refpurpose>
>> + </refnamediv>
>> +
>> + <refentryinfo>
>> + <title>Waffle Manual</title>
>> + <productname>waffle</productname>
>> + <xi:include href="common/author-emil.velikov.xml"/>
>> + <xi:include href="common/copyright.xml"/>
>> + <xi:include href="common/legalnotice.xml"/>
>> + </refentryinfo>
>> +
>> + <refsynopsisdiv>
>> + <funcsynopsis>
>> + <funcsynopsisinfo><![CDATA[#include <waffle.h>]]></funcsynopsisinfo>
>> + <funcprototype>
>> + <funcdef>bool <function>waffle_teardown</function></funcdef>
>> + <void/>
>> + </funcprototype>
>> + </funcsynopsis>
>> + </refsynopsisdiv>
>> +
>> + <refsect1>
>> + <title>Description</title>
>> +
>> + <term><function>waffle_teardown()</function></term>
>> + <listitem>
>> + <para>
>> + Feature test macro: <code>WAFFLE_API_VERSION >= 0x0106</code>.
>> + (See <citerefentry><refentrytitle>waffle_feature_test_macros</refentrytitle><manvolnum>7</manvolnum></citerefentry>).
>> + </para>
>> + <para>
>> + Tears down the per-process global state of the waffle library.
>> + </para>
>> + <para>
>> + А call to <function>waffle_teardown()</function> can fail. In the case it does the caller is advised
>> + to use <function>waffle_error_get_info</function>, <function>waffle_error_get_code</function>
>> + and/or <function>waffle_error_to_string</function> to retrieve the error.
>> +
>> + In case of an error that differs from <errorcode>WAFFLE_ERROR_NOT_INITIALIZED</errorcode> the caller
>> + should not use the Waffle API as the global state is likely to be in an undetermined/corrupt.
>> +
>> + In the case of <errorcode>WAFFLE_ERROR_NOT_INITIALIZED</errorcode> one should call
>> + <function>waffle_init()</function> prior to reusing Waffle.
>> + </para>
>> + </listitem>
>> + </refsect1>
>> +
>> + <refsect1>
>> + <title>Return Value</title>
>> + <xi:include href="common/return-value.xml"/>
>> + </refsect1>
>> +
>> + <refsect1>
>> + <title>Errors</title>
>> +
>> + <xi:include href="common/error-codes.xml"/>
>> +
>> + <variablelist>
>> +
>> + <varlistentry>
>> + <term><errorcode>WAFFLE_ERROR_NOT_INITIALIZED</errorcode></term>
>> + <listitem>
>> + <para>
>> + Waffle has not been initialized with a successfull call to <function>waffle_init()</function>
>> + since the last call to <function>waffle_teardown()</function> or the start of the program.
>> + </para>
>> + </listitem>
>> + </varlistentry>
>> +
>> + </variablelist>
>> +
>> + </refsect1>
>> +
>> + <xi:include href="common/issues.xml"/>
>> +
>> + <refsect1>
>> + <title>See Also</title>
>> +
>> + <para>
>> + <simplelist>
>> + <member><citerefentry><refentrytitle>waffle_init</refentrytitle><manvolnum>3</manvolnum></citerefentry>,</member>
>> + <member><citerefentry><refentrytitle>waffle</refentrytitle><manvolnum>7</manvolnum></citerefentry>.</member>
>> + </simplelist>
>> + </para>
>> + </refsect1>
>> +
>> +</refentry>
>> +
>> +<!--
>> +vim:tw=120 et ts=2 sw=2:
>> +-->
>> diff --git a/src/waffle/api/api_priv.h b/src/waffle/api/api_priv.h
>> index 51cf53c..0e17326 100644
>> --- a/src/waffle/api/api_priv.h
>> +++ b/src/waffle/api/api_priv.h
>> @@ -50,10 +50,10 @@
>> struct api_object;
>> struct wcore_platform;
>>
>> -/// @brief Managed by waffle_init().
>> +/// @brief Managed by waffle_init() and waffle_teardown().
>> ///
>> -/// This is null if and only if waffle has not been initialized with
>> -/// waffle_init().
>> +/// This is null if waffle has not been initialized with waffle_init() or
>> +/// it has been thrown down with waffle_teardown().
>
> I really like the phrase "thrown down" :) It's very colorful. But it should
> be "torn down".
>
Thanks :-) Will fix.
>> extern struct wcore_platform *api_platform;
>>
>> /// @brief Used to validate most API entry points.
>> diff --git a/src/waffle/api/waffle_init.c b/src/waffle/api/waffle_init.c
>> index ebc6cd5..d5b7224 100644
>> --- a/src/waffle/api/waffle_init.c
>> +++ b/src/waffle/api/waffle_init.c
>> @@ -193,3 +193,24 @@ waffle_init(const int32_t *attrib_list)
>>
>> return true;
>> }
>> +
>> +WAFFLE_API bool
>> +waffle_teardown(void)
>> +{
>> + bool ok = true;
>> +
>> + // XXX: Do we really want to reset the error info ?
>> + wcore_error_reset();
>
> Yes, we do. Every public entry point calls wcore_error_reset().
>
>> +
>> + if (!api_platform) {
>> + wcore_error(WAFFLE_ERROR_NOT_INITIALIZED);
>> + return false;
>> + }
>
> I would expect waffle_teardown() to silently succeed if waffle is already
> uninitialized, much like free(NULL) succeeds. But emitting WAFFLE_ERROR_NOT_INITIALIZED
> makes sense too. I'm curious: why did you choose to emit an error here rather than
> succeed? Either behavior makes sense, though, so I'm not suggesting you change it.
>
I'm more of an explicit person (with all of its downsides). Plus the
error was already defined so I opted in to use it.
>> +
>> + ok &= api_platform->vtbl->destroy(api_platform);
>> + if (!ok)
>> + return false;
>
> Now I see why you document in the man page that, if waffle_teardown() emits an error
> other than WAFFLE_ERROR_NOT_INITIALIZED, then future waffle calls have undefined behavior.
> When I wrote the codepaths taken by api_platform->vtbl->destroy, I wasn't considering how
> to gracefully recover from failures and ensure correct behavior of the yet-to-be-written
> waffle_teardown(). There be dragons there.
>
Well if any of the dtors fail (which is quite theoretical), we're in deep s**t.
Thus worrying about recovery is our least concern.
>> +
>> + api_platform = NULL;
>> + return true;
>> +}
>> diff --git a/src/waffle/waffle.def.in b/src/waffle/waffle.def.in
>> index db8464f..7912103 100644
>> --- a/src/waffle/waffle.def.in
>> +++ b/src/waffle/waffle.def.in
>> @@ -6,6 +6,7 @@ EXPORTS
>> waffle_error_to_string
>> waffle_enum_to_string
>> waffle_init
>> + waffle_teardown
>> waffle_make_current
>> waffle_get_proc_address
>> waffle_is_extension_in_string
>>
>
> This all looks good to me. Patch 1 is
> Reviewed-by: Chad Versace <chad.versace at intel.com>
>
Thanks.
I'll take a look at using waffle_teardown in the tests, and
respin/resend when it's looking good.
Cheers,
Emil
More information about the waffle
mailing list