[Mesa-dev] [PATCHv4 2/3] glx_pbuffer: Refactor GetDrawableAttribute

[Jason Wood]

On 02/26/2014 04:27 PM, Adel Gadllah wrote:
> Move the pdraw != NULL check out so that they don't
> have to be duplicated.
>
> Signed-off-by: Adel Gadllah <adel.gadllah at gmail.com>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/glx/glx_pbuffer.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/src/glx/glx_pbuffer.c b/src/glx/glx_pbuffer.c
> index 411d6e5..978730c 100644
> --- a/src/glx/glx_pbuffer.c
> +++ b/src/glx/glx_pbuffer.c
> @@ -350,6 +350,9 @@ GetDrawableAttribute(Display * dpy, GLXDrawable drawable,
>           _XEatData(dpy, length);
>        }
>        else {
> +#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
> +         __GLXDRIdrawable *pdraw = GetGLXDRIDrawable(dpy, drawable);
> +#endif
What is the point of moving the declaration of pdraw into a separate
ifdef from the only one it is used in?  It seems to me that this change
only serves to make the code less readable.

>           _XRead(dpy, (char *) data, length * sizeof(CARD32));
>  
>           /* Search the set of returned attributes for the attribute requested by
> @@ -363,13 +366,11 @@ GetDrawableAttribute(Display * dpy, GLXDrawable drawable,
>           }
>  
>  #if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
> -         {
> -            __GLXDRIdrawable *pdraw = GetGLXDRIDrawable(dpy, drawable);
> -
> -            if (pdraw != NULL && !pdraw->textureTarget)
> +         if (pdraw != NULL) {
> +            if (!pdraw->textureTarget)
>                 pdraw->textureTarget =
>                    determineTextureTarget((const int *) data, num_attributes);
> -            if (pdraw != NULL && !pdraw->textureFormat)
> +            if (!pdraw->textureFormat)
>                 pdraw->textureFormat =
>                    determineTextureFormat((const int *) data, num_attributes);
>           }