Re: [PATCH] glx: Do not call into Composite if it is disabled.

2018-04-10 Thread Adam Jackson
On Wed, 2018-02-14 at 14:33 -0500, Adam Jackson wrote:
> On Tue, 2018-02-13 at 10:33 +0100, Michal Srb wrote:
> > Otherwise X server crashes if GLX is enabled and Composite disabled. For
> > example the compIsAlternateVisual function will try to lookup CompScreenPtr
> > using the CompScreenPrivateKey, but that was never initialized if Composite 
> > is
> > disabled.
> 
> I've hit enough cases like this that I think we should consider
> changing dixLookupPrivate thus:

Well maybe we should, but for now this is fine. Merged, thanks:

remote: I: patch #204141 updated using rev 
1326ee0bc5eb858c3c00847b3ba65134e4ca2e2d.
remote: I: 1 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   b9764b8489..1326ee0bc5  master -> master

- ajax
___
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] glx: Do not call into Composite if it is disabled.

2018-02-14 Thread Adam Jackson
On Tue, 2018-02-13 at 10:33 +0100, Michal Srb wrote:
> Otherwise X server crashes if GLX is enabled and Composite disabled. For
> example the compIsAlternateVisual function will try to lookup CompScreenPtr
> using the CompScreenPrivateKey, but that was never initialized if Composite is
> disabled.

I've hit enough cases like this that I think we should consider
changing dixLookupPrivate thus:

---
diff --git a/include/privates.h b/include/privates.h
index e89c3e440..f34c32fd3 100644
--- a/include/privates.h
+++ b/include/privates.h
@@ -155,12 +155,14 @@ dixSetPrivate(PrivatePtr *privates, const DevPrivateKey 
key, void *val)
  *
  * For privates with defined storage, return the address of the
  * storage. For privates without defined storage, return the pointer
- * contents
+ * contents. If the key is not yet set up return NULL.
  */
 static inline void *
 dixLookupPrivate(PrivatePtr *privates, const DevPrivateKey key)
 {
-if (key->size)
+if (!key->initialized)
+return NULL;
+else if (key->size)
 return dixGetPrivateAddr(privates, key);
 else
 return dixGetPrivate(privates, key);
---

I think if we'd done that I wouldn't have needed to back out v3 of the
"RANDR disabling" patch.

- ajax
___
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] glx: Do not call into Composite if it is disabled.

2018-02-13 Thread Michal Srb
Otherwise X server crashes if GLX is enabled and Composite disabled. For
example the compIsAlternateVisual function will try to lookup CompScreenPtr
using the CompScreenPrivateKey, but that was never initialized if Composite is
disabled.

Fixes: f84e59a4f4. ("glx: Duplicate relevant fbconfigs for compositing visuals")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104993
Signed-off-by: Michal Srb 
---
 glx/glxdricommon.c | 63 +-
 glx/glxscreens.c   | 33 +---
 2 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/glx/glxdricommon.c b/glx/glxdricommon.c
index a16e72849..8747ef545 100644
--- a/glx/glxdricommon.c
+++ b/glx/glxdricommon.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include "extinit.h"
 #include "glxserver.h"
 #include "glxext.h"
 #include "glxcontext.h"
@@ -183,28 +184,30 @@ createModeFromConfig(const __DRIcoreExtension * core,
 config->config.yInverted = GL_TRUE;
 
 #ifdef COMPOSITE
-/*
- * Here we decide what fbconfigs will be duplicated for compositing.
- * fgbconfigs marked with duplicatedForConf will be reserved for
- * compositing visuals.
- * It might look strange to do this decision this late when translation
- * from a __DRIConfig is already done, but using the __DRIConfig
- * accessor function becomes worse both with respect to code complexity
- * and CPU usage.
- */
-if (duplicateForComp &&
-(render_type_is_pbuffer_only(renderType) ||
- config->config.rgbBits != 32 ||
- config->config.redBits != 8 ||
- config->config.greenBits != 8 ||
- config->config.blueBits != 8 ||
- config->config.visualRating != GLX_NONE ||
- config->config.sampleBuffers != 0)) {
-free(config);
-return NULL;
-}
+if (!noCompositeExtension) {
+/*
+* Here we decide what fbconfigs will be duplicated for compositing.
+* fgbconfigs marked with duplicatedForConf will be reserved for
+* compositing visuals.
+* It might look strange to do this decision this late when translation
+* from a __DRIConfig is already done, but using the __DRIConfig
+* accessor function becomes worse both with respect to code complexity
+* and CPU usage.
+*/
+if (duplicateForComp &&
+(render_type_is_pbuffer_only(renderType) ||
+config->config.rgbBits != 32 ||
+config->config.redBits != 8 ||
+config->config.greenBits != 8 ||
+config->config.blueBits != 8 ||
+config->config.visualRating != GLX_NONE ||
+config->config.sampleBuffers != 0)) {
+free(config);
+return NULL;
+}
 
-config->config.duplicatedForComp = duplicateForComp;
+config->config.duplicatedForComp = duplicateForComp;
+}
 #endif
 
 return >config;
@@ -238,14 +241,16 @@ glxConvertConfigs(const __DRIcoreExtension * core,
 }
 
 #ifdef COMPOSITE
-/* Duplicate fbconfigs for use with compositing visuals */
-for (i = 0; configs[i]; i++) {
-tail->next = createModeFromConfig(core, configs[i], GLX_TRUE_COLOR,
-  GL_TRUE);
-if (tail->next == NULL)
-continue;
-
-   tail = tail->next;
+if (!noCompositeExtension) {
+/* Duplicate fbconfigs for use with compositing visuals */
+for (i = 0; configs[i]; i++) {
+tail->next = createModeFromConfig(core, configs[i], GLX_TRUE_COLOR,
+GL_TRUE);
+if (tail->next == NULL)
+continue;
+
+tail = tail->next;
+}
 }
 #endif
 
diff --git a/glx/glxscreens.c b/glx/glxscreens.c
index 596d972e0..5cd9fcfd4 100644
--- a/glx/glxscreens.c
+++ b/glx/glxscreens.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 
+#include "extinit.h"
 #include "privates.h"
 #include "glxserver.h"
 #include "glxutil.h"
@@ -280,10 +281,12 @@ pickFBConfig(__GLXscreen * pGlxScreen, VisualPtr visual)
 if (config->visualID != 0)
 continue;
 #ifdef COMPOSITE
-   /* Use only duplicated configs for compIsAlternateVisuals */
-if (!!compIsAlternateVisual(pGlxScreen->pScreen, visual->vid) !=
-   !!config->duplicatedForComp)
-continue;
+if (!noCompositeExtension) {
+/* Use only duplicated configs for compIsAlternateVisuals */
+if (!!compIsAlternateVisual(pGlxScreen->pScreen, visual->vid) !=
+!!config->duplicatedForComp)
+continue;
+}
 #endif
 /*
  * If possible, use the same swapmethod for all built-in visual
@@ -353,8 +356,10 @@ __glXScreenInit(__GLXscreen * pGlxScreen, ScreenPtr 
pScreen)
 pGlxScreen->visuals[pGlxScreen->numVisuals++] = config;
 config->visualID = visual->vid;
 #ifdef COMPOSITE