Clean up enqueue_map_{buffer,image} tests a bit (#701)

- Introduce MTdataHolder
- Use BufferOwningPtr to manage allocations (fixes a few leaks in error cases)
- Introduce variable for common expressions
- Remove image format support check as the format is required by OpenCL 1.0

Contributes to #700

Signed-off-by: Kévin Petit <kpet@free.fr>

test_common/harness/mt19937.h

@@ -96,4 +96,30 @@ double genrand_res53( MTdata /*data*/ );
     }
 #endif
 
+#ifdef __cplusplus
+
+#include <cassert>
+
+struct MTdataHolder {
+    MTdataHolder(cl_uint seed) {
+        m_mtdata = init_genrand(seed);
+        assert(m_mtdata != nullptr);
+    }
+
+    MTdataHolder(MTdata mtdata) : m_mtdata(mtdata) {}
+
+    ~MTdataHolder() {
+        free_mtdata(m_mtdata);
+    }
+
+    operator MTdata () const {
+        return m_mtdata;
+    }
+
+private:
+    MTdata m_mtdata;
+};
+
+#endif // #ifdef __cplusplus
+
 #endif  /* MT19937_H */

test_conformance/basic/test_enqueue_map.cpp

@@ -45,27 +45,28 @@ int test_enqueue_map_buffer(cl_device_id deviceID, cl_context context, cl_comman
 {
     int error;
     const size_t bufferSize = 256*256;
-    int src_flag_id;
+    MTdataHolder d{gRandomSeed};
-    MTdata d = init_genrand( gRandomSeed );
+    BufferOwningPtr<cl_char> initialData{malloc(bufferSize)};
-    cl_char *initialData = (cl_char*)malloc(bufferSize);
+    BufferOwningPtr<cl_char> finalData{malloc(bufferSize)};
-    cl_char *finalData = (cl_char*)malloc(bufferSize);
 
-    for (src_flag_id=0; src_flag_id < sizeof(flag_set)/sizeof(flag_set[0]); src_flag_id++)
+    for (int src_flag_id=0; src_flag_id < ARRAY_SIZE(flag_set); src_flag_id++)
     {
         clMemWrapper memObject;
         log_info("Testing with cl_mem_flags src: %s\n", flag_set_names[src_flag_id]);
 
         generate_random_data( kChar, (unsigned int)bufferSize, d, initialData );
 
-        if ((flag_set[src_flag_id] & CL_MEM_USE_HOST_PTR) || (flag_set[src_flag_id] & CL_MEM_COPY_HOST_PTR))
+        void *hostPtr = nullptr;
-            memObject = clCreateBuffer(context, flag_set[src_flag_id],  bufferSize * sizeof( cl_char ), initialData, &error);
+        cl_mem_flags flags = flag_set[src_flag_id];
-        else
+        bool hasHostPtr = (flags & CL_MEM_USE_HOST_PTR) || (flags & CL_MEM_COPY_HOST_PTR);
-            memObject = clCreateBuffer(context, flag_set[src_flag_id],  bufferSize * sizeof( cl_char ), NULL, &error);
+        if (hasHostPtr)
+            hostPtr = initialData;
+        memObject = clCreateBuffer(context, flags,  bufferSize, hostPtr, &error);
         test_error( error, "Unable to create testing buffer" );
 
-        if (!(flag_set[src_flag_id] & CL_MEM_USE_HOST_PTR) && !(flag_set[src_flag_id] & CL_MEM_COPY_HOST_PTR))
+        if (!hasHostPtr)
         {
-            error = clEnqueueWriteBuffer(queue, memObject, CL_TRUE, 0, bufferSize * sizeof( cl_char ), initialData, 0, NULL, NULL);
+            error = clEnqueueWriteBuffer(queue, memObject, CL_TRUE, 0, bufferSize, initialData, 0, NULL, NULL);
             test_error( error, "clEnqueueWriteBuffer failed");
         }
 
@@ -81,9 +82,6 @@ int test_enqueue_map_buffer(cl_device_id deviceID, cl_context context, cl_comman
           {
             print_error( error, "clEnqueueMapBuffer call failed" );
             log_error( "\tOffset: %d  Length: %d\n", (int)offset, (int)length );
-            free( initialData );
-            free( finalData );
-            free_mtdata(d);
             return -1;
           }
 
@@ -109,7 +107,7 @@ int test_enqueue_map_buffer(cl_device_id deviceID, cl_context context, cl_comman
         }
 
         // Final validation: read actual values of buffer and compare against our reference
-        error = clEnqueueReadBuffer( queue, memObject, CL_TRUE, 0, sizeof( cl_char ) * bufferSize, finalData, 0, NULL, NULL );
+        error = clEnqueueReadBuffer( queue, memObject, CL_TRUE, 0, bufferSize, finalData, 0, NULL, NULL );
         test_error( error, "Unable to read results" );
 
         for( size_t q = 0; q < bufferSize; q++ )
@@ -117,18 +115,11 @@ int test_enqueue_map_buffer(cl_device_id deviceID, cl_context context, cl_comman
             if( initialData[ q ] != finalData[ q ] )
             {
                 log_error( "ERROR: Sample %d did not validate! Got %d, expected %d\n", (int)q, (int)finalData[ q ], (int)initialData[ q ] );
-                free( initialData );
-                free( finalData );
-                free_mtdata(d);
                 return -1;
             }
         }
     } // cl_mem flags
 
-    free( initialData );
-    free( finalData );
-    free_mtdata(d);
-
     return 0;
 }
 
@@ -137,40 +128,30 @@ int test_enqueue_map_image(cl_device_id deviceID, cl_context context, cl_command
     int error;
     cl_image_format format = { CL_RGBA, CL_UNSIGNED_INT32 };
     const size_t imageSize = 256;
-    int src_flag_id;
+    const size_t imageDataSize = imageSize * imageSize * 4 * sizeof(cl_uint);
-    cl_uint *initialData;
-    cl_uint *finalData;
-    MTdata  d;
 
     PASSIVE_REQUIRE_IMAGE_SUPPORT( deviceID )
 
-    initialData = (cl_uint*)malloc(imageSize * imageSize * 4 *sizeof(cl_uint));
+    BufferOwningPtr<cl_uint> initialData{malloc(imageDataSize)};
-    finalData = (cl_uint*)malloc(imageSize * imageSize * 4 *sizeof(cl_uint));
+    BufferOwningPtr<cl_uint> finalData{malloc(imageDataSize)};
 
-    if( !is_image_format_supported( context, CL_MEM_READ_ONLY, CL_MEM_OBJECT_IMAGE2D, &format ) )
+    MTdataHolder d{gRandomSeed};
-    {
+  for (int src_flag_id=0; src_flag_id < ARRAY_SIZE(flag_set); src_flag_id++) {
-        log_error( "ERROR: Test requires basic OpenCL 1.0 format CL_RGBA:CL_UNSIGNED_INT32, which is unsupported by this device!\n" );
-        free(initialData);
-        free(finalData);
-        return -1;
-    }
-
-    d = init_genrand( gRandomSeed );
-  for (src_flag_id=0; src_flag_id < sizeof(flag_set)/sizeof(flag_set[0]); src_flag_id++) {
     clMemWrapper memObject;
     log_info("Testing with cl_mem_flags src: %s\n", flag_set_names[src_flag_id]);
 
     generate_random_data( kUInt, (unsigned int)( imageSize * imageSize ), d, initialData );
 
-    if ((flag_set[src_flag_id] & CL_MEM_USE_HOST_PTR) || (flag_set[src_flag_id] & CL_MEM_COPY_HOST_PTR))
+    cl_mem_flags flags = flag_set[src_flag_id];
-      memObject = create_image_2d( context, CL_MEM_READ_WRITE | flag_set[src_flag_id], &format,
+    bool hasHostPtr = (flags & CL_MEM_USE_HOST_PTR) || (flags & CL_MEM_COPY_HOST_PTR);
-                                  imageSize, imageSize, 0, initialData, &error );
+    void *hostPtr = nullptr;
-    else
+    if (hasHostPtr)
-      memObject = create_image_2d( context, CL_MEM_READ_WRITE | flag_set[src_flag_id], &format,
+      hostPtr = initialData;
-                                  imageSize, imageSize, 0, NULL, &error );
+    memObject = create_image_2d(context, CL_MEM_READ_WRITE | flags, &format,
+                                imageSize, imageSize, 0, hostPtr, &error );
     test_error( error, "Unable to create testing buffer" );
 
-    if (!(flag_set[src_flag_id] & CL_MEM_USE_HOST_PTR) && !(flag_set[src_flag_id] & CL_MEM_COPY_HOST_PTR)) {
+    if (!hasHostPtr) {
       size_t write_origin[3]={0,0,0}, write_region[3]={imageSize, imageSize, 1};
       error = clEnqueueWriteImage(queue, memObject, CL_TRUE, write_origin, write_region, 0, 0, initialData, 0, NULL, NULL);
       test_error( error, "Unable to write to testing buffer" );
@@ -194,9 +175,6 @@ int test_enqueue_map_image(cl_device_id deviceID, cl_context context, cl_command
       {
         print_error( error, "clEnqueueMapImage call failed" );
         log_error( "\tOffset: %d,%d  Region: %d,%d\n", (int)offset[0], (int)offset[1], (int)region[0], (int)region[1] );
-        free(initialData);
-        free(finalData);
-        free_mtdata(d);
         return -1;
       }
 
@@ -237,18 +215,11 @@ int test_enqueue_map_image(cl_device_id deviceID, cl_context context, cl_command
       {
         log_error( "ERROR: Sample %d (coord %d,%d) did not validate! Got %d, expected %d\n", (int)q, (int)( ( q / 4 ) % imageSize ), (int)( ( q / 4 ) / imageSize ),
                                     (int)finalData[ q ], (int)initialData[ q ] );
-        free(initialData);
-        free(finalData);
-        free_mtdata(d);
         return -1;
       }
     }
   } // cl_mem_flags
 
-    free(initialData);
-    free(finalData);
-    free_mtdata(d);
     return 0;
 }
 
-