Topi Pohjolainen <topi.pohjolai...@intel.com> writes:
> Signed-off-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
> ---
> src/mesa/drivers/dri/intel/intel_screen.c | 101
> +++++++++++++++++++++---------
> 1 file changed, 71 insertions(+), 30 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_screen.c
> b/src/mesa/drivers/dri/intel/intel_screen.c
> index 3d0344e..8716688 100644
> --- a/src/mesa/drivers/dri/intel/intel_screen.c
> +++ b/src/mesa/drivers/dri/intel/intel_screen.c
> @@ -496,8 +496,14 @@ intel_create_image_from_texture(__DRIcontext *context,
> int target,
> static void
> intel_destroy_image(__DRIimage *image)
> {
> - intel_region_release(&image->regions[0]);
> - free(image);
> + int i;
> +
> + for (i = 0; i < sizeof(image->regions) / sizeof(struct intel_region);
> ++i) {
All instances of this should be using ARRAY_SIZE.
> + if (image->regions[i]->bo)
> + intel_region_release(&image->regions[i]);
Why are you looking at the BO for deciding whether to free the region?
A region should always have a BO. If you're trying to null-check the
region, intel_region_release already does that.
> + for (i = 0; i < sizeof(image->regions) / sizeof(struct intel_region);
> ++i) {
> + if (!orig_image->regions[i]->bo)
> + break;
null-checking the wrong thing.
> +
> + intel_region_reference(&image->regions[i], orig_image->regions[i]);
> + if (image->regions[i] == NULL) {
> + for (--i; i >= 0; --i)
> + intel_region_release(&image->regions[i]);
> +
> + free(image);
mixed tab-and-space indentation, should always be spaces.
> + return NULL;
> + }
> }
>
> image->internal_format = orig_image->internal_format;
> @@ -649,45 +664,71 @@ intel_create_image_from_names(__DRIscreen *screen,
> }
>
> static __DRIimage *
> +intel_setup_image_from_fds(struct intel_screen *screen, int width, int
> height,
> + const struct intel_image_format *f,
> + const int *fds, int num_fds, const int *strides,
> + void *loaderPriv)
> +{
> + int i;
> + __DRIimage *img = intel_allocate_image(__DRI_IMAGE_FORMAT_NONE,
> loaderPriv);
> +
> + if (img == NULL)
> + return NULL;
> +
> + for (i = 0; i < num_fds; i++) {
> + img->regions[i] = intel_region_alloc_for_fd(screen, f->planes[i].cpp,
> + width >> f->planes[i].width_shift,
> + height >> f->planes[i].height_shift,
> + strides[i], fds[i], "image");
> +
> + if (img->regions[i] == NULL) {
> + for (--i; i >= 0; --i)
> + intel_region_release(&img->regions[i]);
> +
> + free(img);
> + return NULL;
> + }
> + }
I've seen this image-freeing loop a few times, seems like it should be
in just one place.
pgpXCaHu8RZ4E.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
