[Mesa-dev] [PATCH 2/4 v2] mesa/image: assert on bad format

[nobled]

NULL as an error indicator is meaningless, since it will return NULL
on success anyway if the caller passes in zero as the image's address
and asks to calculate the offset of the first pixel. For example,
_mesa_validate_pbo_access() does this.

This also matches the code in the non-GL_BITMAP codepath, which
already has an assert like this.

v2: Per Brian Paul's review, remove the function call entirely
and tighten the assert to only accept the two formats compatible with
GL_BITMAP. They always have one component per pixel.
---
 src/mesa/main/image.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/mesa/main/image.c b/src/mesa/main/image.c
index ca5771c..af96107 100644
--- a/src/mesa/main/image.c
+++ b/src/mesa/main/image.c
@@ -1093,17 +1093,17 @@ _mesa_is_compressed_format(struct gl_context
*ctx, GLenum format)
  * Pixel unpacking/packing parameters are observed according to \p packing.
  *
  * \param dimensions either 1, 2 or 3 to indicate dimensionality of image
+ * \param packing  the pixelstore attributes
  * \param image  starting address of image data
  * \param width  the image width
- * \param height  theimage height
- * \param format  the pixel format
- * \param type  the pixel data type
- * \param packing  the pixelstore attributes
+ * \param height  the image height
+ * \param format  the pixel format (must be validated beforehand)
+ * \param type  the pixel data type (must be validated beforehand)
  * \param img  which image in the volume (0 for 1D or 2D images)
  * \param row  row of pixel in the image (0 for 1D images)
  * \param column column of pixel in the image
  *
- * \return address of pixel on success, or NULL on error.
+ * \return address of pixel.
  *
  * \sa gl_pixelstore_attrib.
  */
@@ -1147,15 +1147,13 @@ _mesa_image_address( GLuint dimensions,

    if (type == GL_BITMAP) {
       /* BITMAP data */
-      GLint comp_per_pixel;   /* components per pixel */
       GLint bytes_per_row;
       GLint bytes_per_image;
+      /* components per pixel for color or stencil index: */
+      const GLint comp_per_pixel = 1;

-      /* Compute number of components per pixel */
-      comp_per_pixel = _mesa_components_in_format( format );
-      if (comp_per_pixel < 0) {
-         return NULL;
-      }
+      /* The pixel type and format should have been error checked earlier */
+      assert(format == GL_COLOR_INDEX || format == GL_STENCIL_INDEX);

       bytes_per_row = alignment
                     * CEILING( comp_per_pixel*pixels_per_row, 8*alignment );
-- 
1.7.6.msysgit.0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-mesa-image-assert-on-bad-format.patch
Type: application/octet-stream
Size: 2784 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111019/de13f77c/attachment-0001.obj>