[Mesa-dev] [PATCH 4/8] intel: Implement __DRIimage::createSubImage and bump supported version to 5

[Kristian Høgsberg]

On Fri, Jul 06, 2012 at 02:16:23PM -0700, Eric Anholt wrote:
> Kristian Høgsberg <krh at bitplanet.net> writes:
> > +static __DRIimage *
> > +intel_create_sub_image(__DRIimage *parent,
> > +                       int width, int height, int dri_format,
> > +                       int offset, int pitch, void *loaderPrivate)
> > +{
> > +    __DRIimage *image;
> > +
> > +    image = intel_allocate_image(dri_format, loaderPrivate);
> > +
> > +    image->region = calloc(sizeof(*image->region), 1);
> > +    if (image->region == NULL) {
> > +       FREE(image);
> > +       return NULL;
> > +    }
> > +
> > +    image->region->cpp = _mesa_get_format_bytes(image->format);
> > +    image->region->width = width;
> > +    image->region->height = height;
> > +    image->region->pitch = pitch;
> > +    image->region->refcount = 1;
> > +    image->region->bo = parent->region->bo;
> > +    drm_intel_bo_reference(image->region->bo);
> > +    image->region->tiling = parent->region->tiling;
> > +    image->offset = offset;
> > +
> > +    return image;
> > +}
> 
> I would be a lot more comfortable if there was some validation that
> offset + height * pitch was within the buffer object referenced, at
> least.

That's a good point.  We should probably validate that for
intel_region_alloc_for_handle() too (that is, that height * pitch <=
bo-size).

> There's also (offset % tile_width) != 0 possibility, but at
> least it wouldn't involve rendering outside the BO afaik.

Yeah, maybe just _mesa_warning for that one?  Getting a corrupted
output is immediately noticable and easier to track down than just
returning NULL and getting a black texture.
 
> image->region->screen isn't set, so if intel_query_image() is callable
> on it, it would segfault.  Seems like that ought to be fixed.

Ah yes, will add.

Kristian