Am 27.02.2014 03:14, schrieb Jason Wood:
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...


Well I could move it to the top in the second patch as well but the thing is it does not make much sense to move it to the top while being only used in the else branch ...

Patch 3/3 moves it to the top because it does not have to do the round trip 
(everything can be done at the client side).

So its not really "extra code churn" but changes that more or less make sense 
in isolation.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to