[Mesa-dev] [PATCH] mesa/pbo: Handle zero height or depth when validating PBO access

[Neil Roberts]

Ilia Mirkin <imirkin at alum.mit.edu> writes:

>> -   end =  _mesa_image_offset(dimensions, pack, width, height,
>> -                             format, type, depth-1, height-1, width);
>> +   if (depth == 0 || height == 0)
>
> Why not width == 0 as well? You could probably just do
>
>   return GL_TRUE;
>
> in that case as well, and then call _mesa_image_offset unconditionally
> for both end/start. IMHO simpler and more efficient.

Yes, that makes sense. I was thinking we would want to preserve the
error in the case where the start address is out of range even if the
size is zero because that would be an especially weird thing for the
application to do and maybe an error would be helpful. However the spec
doesn't imply that this should happen and it only says:

 “An INVALID_OPERATION error is generated if a pixel unpack buffer
  object is bound and unpacking the pixel data according to the process
  described below would access memory beyond the size of the pixel
  unpack buffer’s memory size.”

I'll make a V2.

>> +      end = start;
>> +   else
>> +      end =  _mesa_image_offset(dimensions, pack, width, height,
>
> I've always found a single space is enough for me... apparently not
> for the original author of this code (which you then just indented).

Yes, I noticed this too but I didn't change it because then I was in a
dilemma about whether I should put spaces around the - operator too I
decided it was easier to just leave the whole line as it is :)

Thanks for the review.

Regards,
- Neil