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

[Chad Versace]

On 08/19/2013 10:45 AM, Chad Versace wrote:
> 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.
>