[Mesa-dev] [PATCH] st/omx: straighten get/put_screen

[Emil Velikov]

The current code is busted in a number of ways.

 - initially checks for omx_display (rather than omx_screen), which may
or may not be around.
 - blindly feeds the empty env variable string to loader_open_device()
 - reads the env variable every time get_screen is called
 - the latter manifests into memory leaks, and other issues as one sets
the variable between two get_screen calls.

Additionally it cleans up a couple of extra bits
 - drops unneeded set/check of omx_display.
 - make the teardown (put_screen) order was not symmetrical to the setup
(get_screen)

Cc: Leo Liu <leo.liu at amd.com>
Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
---

Leo, feel free to point out if I'm loosing the plot and some of these 
don't hold true.

-Emil

 src/gallium/state_trackers/omx/entrypoint.c | 35 ++++++++++++++++-------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/src/gallium/state_trackers/omx/entrypoint.c b/src/gallium/state_trackers/omx/entrypoint.c
index 7df90b1..f99a620 100644
--- a/src/gallium/state_trackers/omx/entrypoint.c
+++ b/src/gallium/state_trackers/omx/entrypoint.c
@@ -33,6 +33,7 @@
 
 #include <assert.h>
 #include <string.h>
+#include <stdbool.h>
 
 #include <X11/Xlib.h>
 
@@ -73,28 +74,32 @@ int omx_component_library_Setup(stLoaderComponentType **stComponents)
 
 struct vl_screen *omx_get_screen(void)
 {
+   static bool first_time = true;
    pipe_mutex_lock(omx_lock);
 
-   if (!omx_display) {
-      omx_render_node = debug_get_option("OMX_RENDER_NODE", NULL);
-      if (!omx_render_node) {
-         omx_display = XOpenDisplay(NULL);
-         if (!omx_display)
-            goto error;
-      }
-   }
-
    if (!omx_screen) {
+      if (first_time) {
+         omx_render_node = debug_get_option("OMX_RENDER_NODE", NULL);
+         if (omx_render_node && *omx_render_node == '\0')
+            omx_render_node = NULL;
+
+         first_time = false;
+      }
       if (omx_render_node) {
          drm_fd = loader_open_device(omx_render_node);
          if (drm_fd < 0)
             goto error;
+
          omx_screen = vl_drm_screen_create(drm_fd);
          if (!omx_screen) {
             close(drm_fd);
             goto error;
          }
       } else {
+         omx_display = XOpenDisplay(NULL);
+         if (!omx_display)
+            goto error;
+
          omx_screen = vl_screen_create(omx_display, 0);
          if (!omx_screen) {
             XCloseDisplay(omx_display);
@@ -117,16 +122,14 @@ void omx_put_screen(void)
 {
    pipe_mutex_lock(omx_lock);
    if ((--omx_usecount) == 0) {
-      if (!omx_render_node) {
-         vl_screen_destroy(omx_screen);
-         if (omx_display)
-            XCloseDisplay(omx_display);
-      } else {
-         close(drm_fd);
+      if (omx_render_node) {
          vl_drm_screen_destroy(omx_screen);
+         close(drm_fd);
+      } else {
+         vl_screen_destroy(omx_screen);
+         XCloseDisplay(omx_display);
       }
       omx_screen = NULL;
-      omx_display = NULL;
    }
    pipe_mutex_unlock(omx_lock);
 }
-- 
2.6.2