[Mesa-dev] [PATCH 09/14] mesa_glinterop: fix GL interop *_VERSION comments

[Marek Olšák]

On Tue, May 24, 2016 at 4:32 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
>
> Using the macro to set the version is wrong and ill-advised. Please don't
> do it.
>
> Cc: Marek Olšák <marek.olsak at amd.com>
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
> Marek, now things start to unravel as to why you were not too excited on
> the idea. Hopefully this comment makes things clearer/more descriptive.
> If not please let me know how we can improve it.
> ---
>  include/GL/mesa_glinterop.h | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/include/GL/mesa_glinterop.h b/include/GL/mesa_glinterop.h
> index 5c172c6..f637409 100644
> --- a/include/GL/mesa_glinterop.h
> +++ b/include/GL/mesa_glinterop.h
> @@ -93,7 +93,8 @@ enum {
>   * Device information returned by Mesa.
>   */
>  typedef struct _mesa_glinterop_device_info {
> -   /* The caller should set this to: MESA_GLINTEROP_DEVICE_INFO_VERSION */
> +   /* The caller should set this to the version of the struct they support */
> +   /* NOTE: Do not use the MESA_GLINTEROP_DEVICE_INFO_VERSION macro */
>     uint32_t struct_version;
>
>     /* PCI location */
> @@ -124,7 +125,8 @@ typedef struct _mesa_glinterop_device_info {
>   * Input parameters to Mesa interop export functions.
>   */
>  typedef struct _mesa_glinterop_export_in {
> -   /* The caller should set this to: MESA_GLINTEROP_EXPORT_IN_VERSION */
> +   /* The caller should set this to the version of the struct they support */
> +   /* NOTE: Do not use the MESA_GLINTEROP_EXPORT_IN_VERSION macro */
>     uint32_t struct_version;
>
>     /* One of the following:
> @@ -183,7 +185,8 @@ typedef struct _mesa_glinterop_export_in {
>   * Outputs of Mesa interop export functions.
>   */
>  typedef struct _mesa_glinterop_export_out {
> -   /* The caller should set this to: MESA_GLINTEROP_EXPORT_OUT_VERSION */
> +   /* The caller should set this to the version of the struct they support */
> +   /* NOTE: Do not use the MESA_GLINTEROP_EXPORT_OUT_VERSION macro */
>     uint32_t struct_version;

Obviously, the comments don't apply to closed-source projects or any
open source projects that just import the header and use the interface
correctly and completely. If that's the case, using the VERSION macros
makes sense. It had never been my intention for other projects to get
this header from /usr/include/GL. Maybe we shouldn't install it.

This patch would be OK if the header was used from /usr/include/GL
only. Since that's not the expected usage, this patch is NAK.

Marek