vk/image: Refactor anv_image_create()
authorChad Versace <chad.versace@intel.com>
Fri, 26 Jun 2015 16:17:52 +0000 (09:17 -0700)
committerChad Versace <chad.versace@intel.com>
Fri, 26 Jun 2015 16:32:59 +0000 (09:32 -0700)
From my experience with intel_mipmap_tree.c, I learned that for struct's
like anv_image and intel_mipmap_tree, which have sprawling
multi-function construction codepaths, it's easy to mistakenly use
unitialized struct members during construction.

Let's eliminate the risk of using unitialized anv_image members during
construction.  Fill the struct at the function bottom instead of
piecemeal throughout the constructor.

src/vulkan/image.c

index 607454fb498b02d79a86cd6c5d40fca1c179c068..c8a26f661300412656407a37602e137971d44147 100644 (file)
@@ -115,18 +115,10 @@ VkResult anv_image_create(
     VkImage*                                    pImage)
 {
    struct anv_device *device = (struct anv_device *) _device;
-   struct anv_image *image;
-   const struct anv_format *format_info;
-   int32_t aligned_height;
-   uint32_t stencil_size;
+   const VkExtent3D *restrict extent = &pCreateInfo->extent;
 
    assert(pCreateInfo->sType == VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO);
 
-   image = anv_device_alloc(device, sizeof(*image), 8,
-                            VK_SYSTEM_ALLOC_TYPE_API_OBJECT);
-   if (image == NULL)
-      return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
-
    /* XXX: We don't handle any of these */
    anv_assert(pCreateInfo->imageType == VK_IMAGE_TYPE_2D);
    anv_assert(pCreateInfo->mipLevels == 1);
@@ -136,55 +128,48 @@ VkResult anv_image_create(
    anv_assert(pCreateInfo->extent.height > 0);
    anv_assert(pCreateInfo->extent.depth == 1);
 
-   image->bo = NULL;
-   image->offset = 0;
-   image->type = pCreateInfo->imageType;
-   image->format = pCreateInfo->format;
-   image->extent = pCreateInfo->extent;
-   image->swap_chain = NULL;
-   image->tile_mode = anv_image_choose_tile_mode(pCreateInfo, extra);
+   const uint32_t tile_mode =
+      anv_image_choose_tile_mode(pCreateInfo, extra);
 
    /* TODO(chadv): How should we validate inputs? */
-   image->surf_type = anv_surf_type_from_image_type[pCreateInfo->imageType];
+   const uint8_t surf_type =
+      anv_surf_type_from_image_type[pCreateInfo->imageType];
 
    const struct anv_surf_type_limits *limits =
-      &anv_surf_type_limits[image->surf_type];
+      &anv_surf_type_limits[surf_type];
 
    const struct anv_tile_info *tile_info =
-       &anv_tile_info_table[image->tile_mode];
-
-   if (image->extent.width > limits->width ||
-       image->extent.height > limits->height ||
-       image->extent.depth > limits->depth) {
-      anv_loge("image extent is too large");
-      free(image);
+       &anv_tile_info_table[tile_mode];
 
+   if (extent->width > limits->width ||
+       extent->height > limits->height ||
+       extent->depth > limits->depth) {
       /* TODO(chadv): What is the correct error? */
+      anv_loge("image extent is too large");
       return vk_error(VK_ERROR_INVALID_MEMORY_SIZE);
    }
 
-   image->alignment = tile_info->surface_alignment;
-
-   /* FINISHME: Stop hardcoding miptree image alignment */
-   image->h_align = 4;
-   image->v_align = 4;
-
-   format_info = anv_format_for_vk_format(pCreateInfo->format);
+   const struct anv_format *format_info =
+      anv_format_for_vk_format(pCreateInfo->format);
    assert(format_info->cpp > 0 || format_info->has_stencil);
 
+   uint32_t image_stride = 0;
+   uint32_t image_size = 0;
+   uint32_t stencil_offset = 0;
+   uint32_t stencil_stride = 0;
+
    /* First allocate space for the color or depth buffer. info->cpp gives us
     * the cpp of the color or depth in case of depth/stencil formats. Stencil
     * only (VK_FORMAT_S8_UINT) has info->cpp == 0 and doesn't allocate
     * anything here.
     */
    if (format_info->cpp > 0) {
-      image->stride = ALIGN_I32(image->extent.width * format_info->cpp,
-                                tile_info->width);
-      aligned_height = ALIGN_I32(image->extent.height, tile_info->height);
-      image->size = image->stride * aligned_height;
-   } else {
-      image->size = 0;
-      image->stride = 0;
+      uint32_t aligned_height;
+
+      image_stride = ALIGN_I32(extent->width * format_info->cpp,
+                               tile_info->width);
+      aligned_height = ALIGN_I32(extent->height, tile_info->height);
+      image_size = image_stride * aligned_height;
    }
 
    /* Formats with a stencil buffer (either combined depth/stencil or
@@ -195,16 +180,50 @@ VkResult anv_image_create(
     */
    if (format_info->has_stencil) {
       const struct anv_tile_info *w_info = &anv_tile_info_table[WMAJOR];
-      image->stencil_offset = ALIGN_U32(image->size, w_info->surface_alignment);
-      image->stencil_stride = ALIGN_I32(image->extent.width, w_info->width);
-      aligned_height = ALIGN_I32(image->extent.height, w_info->height);
-      stencil_size = image->stencil_stride * aligned_height;
-      image->size = image->stencil_offset + stencil_size;
-   } else {
-      image->stencil_offset = 0;
-      image->stencil_stride = 0;
+      uint32_t aligned_height;
+      uint32_t stencil_size;
+
+      stencil_offset = ALIGN_U32(image_size, w_info->surface_alignment);
+      stencil_stride = ALIGN_I32(extent->width, w_info->width);
+      aligned_height = ALIGN_I32(extent->height, w_info->height);
+      stencil_size = stencil_stride * aligned_height;
+      image_size = stencil_offset + stencil_size;
    }
 
+   struct anv_image *image = anv_device_alloc(device, sizeof(*image), 8,
+                                              VK_SYSTEM_ALLOC_TYPE_API_OBJECT);
+   if (!image)
+      return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
+
+   /* To eliminate the risk of using unitialized struct members above, fill the
+    * image struct here at the function bottom instead of piecemeal throughout
+    * the function body.
+    */
+   *image = (struct anv_image) {
+      .type = pCreateInfo->imageType,
+      .extent = pCreateInfo->extent,
+      .format = pCreateInfo->format,
+
+      .size = image_size,
+      .alignment = tile_info->surface_alignment,
+      .stride = image_stride,
+
+      .bo = NULL,
+      .offset = 0,
+
+      .stencil_offset = stencil_offset,
+      .stencil_stride = stencil_stride,
+
+      .tile_mode = tile_mode,
+      .surf_type = surf_type,
+
+      /* FINISHME: Stop hardcoding miptree image alignment */
+      .h_align = 4,
+      .v_align = 4,
+
+      .swap_chain = NULL,
+   };
+
    *pImage = (VkImage) image;
 
    return VK_SUCCESS;