On 02/26/2014 06:55 PM, Ian Romanick wrote:
> On 02/26/2014 05:22 PM, Jason Wood wrote:
>> 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.gadl...@gmail.com>
>>> Reviewed-by: Ian Romanick <ian.d.roman...@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.
>
> This was my recommendation. It eliminates needing to add yet another
> level of indentation below... especially with patch 3/3 on top of it.
That is fair. I should have taken a closer look at 3/3 before commenting.
However, patch 3/3 again moves the declaration for pdraw -- to the top
of the function. Why move the declaration twice? No need for the extra
code churn...
>
>>> _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);
>>> }
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
