Re: [PATCH xserver] composite: Make compIsAlternateVisual safe even if Composite is off

[walter harms]

Am 27.07.2017 22:02, schrieb Adam Jackson:
> As of ea483af9 we're calling this unconditionally from the GLX code so
> the synthetic visual is in a lower select group. If Composite has been
> disabled then GetCompScreen() will return NULL, and this would crash.
> 
> Rather than force the caller to check first, just always return FALSE if
> Composite is disabled (which is correct, since none of the visuals will
> be synthetic in that case).
> 
> Signed-off-by: Adam Jackson 
> ---
>  composite/compwindow.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/composite/compwindow.c b/composite/compwindow.c
> index 367f23eb7..f88238146 100644
> --- a/composite/compwindow.c
> +++ b/composite/compwindow.c
> @@ -326,7 +326,7 @@ compIsAlternateVisual(ScreenPtr pScreen, XID visual)
>  CompScreenPtr cs = GetCompScreen(pScreen);
>  int i;
>  
> -for (i = 0; i < cs->numAlternateVisuals; i++)
> +for (i = 0; cs && i < cs->numAlternateVisuals; i++)
>  if (cs->alternateVisuals[i] == visual)
>  return TRUE;
>  return FALSE;

IMHO this would be more readable:

cs = GetCompScreen(pScreen);
if (!cs)
return FALSE;

   for()...


just my 2 cents ...

re,
 wh
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] composite: Make compIsAlternateVisual safe even if Composite is off

[Aaron Plattner]

On 07/27/2017 01:02 PM, Adam Jackson wrote:

As of ea483af9 we're calling this unconditionally from the GLX code so
the synthetic visual is in a lower select group. If Composite has been
disabled then GetCompScreen() will return NULL, and this would crash.

Rather than force the caller to check first, just always return FALSE if
Composite is disabled (which is correct, since none of the visuals will
be synthetic in that case).

Signed-off-by: Adam Jackson 
---
  composite/compwindow.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/composite/compwindow.c b/composite/compwindow.c
index 367f23eb7..f88238146 100644
--- a/composite/compwindow.c
+++ b/composite/compwindow.c
@@ -326,7 +326,7 @@ compIsAlternateVisual(ScreenPtr pScreen, XID visual)
  CompScreenPtr cs = GetCompScreen(pScreen);
  int i;
  
-for (i = 0; i < cs->numAlternateVisuals; i++)

+for (i = 0; cs && i < cs->numAlternateVisuals; i++)


I really hope gcc is smart enough to know that cs won't change here and 
this check can be hoisted out of the loop.



  if (cs->alternateVisuals[i] == visual)
  return TRUE;
  return FALSE;



Sure, can't hurt.

Reviewed-by: Aaron Plattner 

--
nvpublic
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver] composite: Make compIsAlternateVisual safe even if Composite is off

[Adam Jackson]

As of ea483af9 we're calling this unconditionally from the GLX code so
the synthetic visual is in a lower select group. If Composite has been
disabled then GetCompScreen() will return NULL, and this would crash.

Rather than force the caller to check first, just always return FALSE if
Composite is disabled (which is correct, since none of the visuals will
be synthetic in that case).

Signed-off-by: Adam Jackson 
---
 composite/compwindow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/composite/compwindow.c b/composite/compwindow.c
index 367f23eb7..f88238146 100644
--- a/composite/compwindow.c
+++ b/composite/compwindow.c
@@ -326,7 +326,7 @@ compIsAlternateVisual(ScreenPtr pScreen, XID visual)
 CompScreenPtr cs = GetCompScreen(pScreen);
 int i;
 
-for (i = 0; i < cs->numAlternateVisuals; i++)
+for (i = 0; cs && i < cs->numAlternateVisuals; i++)
 if (cs->alternateVisuals[i] == visual)
 return TRUE;
 return FALSE;
-- 
2.13.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel