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

[Ian Romanick]

On 10/19/2011 05:10 PM, nobled wrote:
> 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.

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> ---
>   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 );
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev