[waffle] [PATCH 1/3] waffle: add public func waffle_teardown()

Chad Versace chad.versace at intel.com
Tue Jan 20 10:28:37 PST 2015


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".

>  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.

> +
> +    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.

> +
> +    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>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 884 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/waffle/attachments/20150120/57bd8551/attachment.sig>


More information about the waffle mailing list