[Piglit] [v9 09/13] tests/spec: EXT_image_dma_buf_import fd ownership transfer

[Chad Versace]

On 08/09/2013 03:43 AM, Topi Pohjolainen wrote:
> Simple test checking that EGL closes the export file handle and
> the creator can in turn drop its reference.
>
> v2:
>     - compile only on platforms that have drm (Eric)
>     - use standard drm definitions for fourcc instead of duplicated
>       local (Daniel, Eric)
>     - use helper variables for width, height and cpp instead of
>       repeating the magic numbers over and over again (Eric)
>     - try to close the export file descriptor and check that it is
>       already closed by the EGL stack (Eric, Chad)
>     - fix typo in the description (and commit) (Chad)
>     - renamed from "close_buffer" to "ownership_transfer"
>     - removed irrelevant quote of the spec (Eric)
>
> v3:
>     - use properly linked egl-extension calls (Eric)
>     - check for EBADF and not just for close()-failure (Daniel)
>
> v4 (Eric):
>     - add to 'all.tests'
>     - removed inclusion of standard EGL entrypoints as there is the
>       special dispatcher for the test cases to use
>     - close the egl-display before checking if the file descriptor
>       is closed
>
> v5 (Chad):
>     - skip the test if EGL does not support the chosen format
>     - report possible failure of 'eglTerminate()' in the console
>

> +static enum piglit_result
> +test_create_and_destroy(unsigned w, unsigned h, void *buf, int fd,
> +		unsigned stride, unsigned offset)
> +{

...

> +	img = eglCreateImageKHR(eglGetCurrentDisplay(), EGL_NO_CONTEXT,
> +			EGL_LINUX_DMA_BUF_EXT, (EGLClientBuffer)0, attr);
> +
> +	/* Release the creator side of the buffer. */
> +	piglit_destroy_dma_buf(buf);
> +
> +	/* EGL may not support the format, this is not an error. */
> +	if (!img && piglit_check_egl_error(EGL_BAD_MATCH))
> +		return PIGLIT_SKIP;
> +
> +	if (!piglit_check_egl_error(EGL_SUCCESS))
> +		return PIGLIT_FAIL;
> +
> +	if (!img) {
> +		fprintf(stderr, "image creation succeed but returned NULL\n");
> +		return PIGLIT_FAIL;
> +	}

Regarding this error checking, the code's intent is correct, but the details are not.
piglit_check_egl_error() calls eglGetError(), and eglGetError() *resets* EGL's error
state to EGL_SUCCESS. So, if !img, then the second call to piglit_check_egl_error() always succeeds!

Either (1) get the error code by calling eglGetError() exactly once, and then handle it manually yourself without
piglit_check_egl_error(), or (2) restructure the control flow to ensure that piglit_check_egl_error() is called
no more than once.

Other than that, this patch looks good.