From: Orestis Floros <orestisf1993@gmail.com>
Date: Tue, 11 Sep 2018 16:11:05 +0000 (+0300)
Subject: init_ws_for_output: use workspace_move_to_output
X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=4d21f4cfc2cfd3166164218f665f89b9ec8a69a5;p=i3%2Fi3

init_ws_for_output: use workspace_move_to_output

This fixes a crash produced with the following config:
    # i3 config file (v4)
    workspace 1 output $screen1
    workspace 2 output $screen2

    exec --no-startup-id "i3-msg workspace 1, open && i3-msg workspace 2 && xrandr --output $screen2 --off && xrandr --output $screen1 --auto --output $screen2 --auto --right-of $screen1 "

Which results in:
ERROR: AddressSanitizer: heap-use-after-free on address …
READ of size 8 at 0x614000001f48 thread T0
    #0 0x5563df6e73a8 in init_ws_for_output i3/src/randr.c:468
    #1 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940
    #2 0x5563df68dbe1 in handle_screen_change i3/src/handlers.c:450

… is located 264 bytes inside of 448-byte region …
freed by thread T0 here:
    #1 0x5563df634b0a in con_free i3/src/con.c:96
    #2 0x5563df7151e6 in tree_close_internal i3/src/tree.c:344
    #3 0x5563df7280fe in workspace_show i3/src/workspace.c:499
    #4 0x5563df6e7315 in init_ws_for_output i3/src/randr.c:457
    #5 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940
    #6 0x5563df68dbe1 in handle_screen_change i3/src/handlers.c:450

Which is similar to #3228, #3248.
---

diff --git a/src/randr.c b/src/randr.c
index b1a9e8cb..0c1f9704 100644
--- a/src/randr.c
+++ b/src/randr.c
@@ -442,47 +442,13 @@ void init_ws_for_output(Output *output, Con *content) {
             continue;
         }
 
-        /* if so, move it over */
-        LOG("Moving workspace \"%s\" from output \"%s\" to \"%s\" due to assignment\n",
-            workspace->name, workspace_out->name, output_primary_name(output));
-
-        /* if the workspace is currently visible on that output, we need to
-         * switch to a different workspace - otherwise the output would end up
-         * with no active workspace */
-        bool visible = workspace_is_visible(workspace);
-        Con *previous = NULL;
-        if (visible && (previous = TAILQ_NEXT(workspace, focused))) {
-            LOG("Switching to previously used workspace \"%s\" on output \"%s\"\n",
-                previous->name, workspace_out->name);
-            workspace_show(previous);
-        }
-
-        /* Render the output on which the workspace was to get correct Rects.
-         * Then, we need to work with the "content" container, since we cannot
-         * be sure that the workspace itself was rendered at all (in case it’s
-         * invisible, it won’t be rendered). */
-        render_con(workspace_out, false);
-        Con *ws_out_content = output_get_content(workspace_out);
-
-        Con *floating_con;
-        TAILQ_FOREACH(floating_con, &(workspace->floating_head), floating_windows)
-        /* NB: We use output->con here because content is not yet rendered,
-             * so it has a rect of {0, 0, 0, 0}. */
-        floating_fix_coordinates(floating_con, &(ws_out_content->rect), &(output->con->rect));
-
-        con_detach(workspace);
-        con_attach(workspace, content, false);
-
-        /* In case the workspace we just moved was visible but there was no
-         * other workspace to switch to, we need to initialize the source
-         * output as well */
-        if (visible && previous == NULL) {
-            LOG("There is no workspace left on \"%s\", re-initializing\n",
-                workspace_out->name);
-            init_ws_for_output(get_output_by_name(workspace_out->name, true),
-                               output_get_content(workspace_out));
-            DLOG("Done re-initializing, continuing with \"%s\"\n", output_primary_name(output));
-        }
+        DLOG("Moving workspace \"%s\" from output \"%s\" to \"%s\" due to assignment\n",
+             workspace->name, workspace_out->name, output_primary_name(output));
+        /* Need to copy output's rect since content is not yet rendered. We
+         * can't call render_con here because render_output only proceeds if a
+         * workspace exists. */
+        content->rect = output->con->rect;
+        workspace_move_to_output(workspace, output);
     }
 
     /* if a workspace exists, we are done now */
diff --git a/src/workspace.c b/src/workspace.c
index e92aaa5f..2a9f88db 100644
--- a/src/workspace.c
+++ b/src/workspace.c
@@ -966,7 +966,11 @@ bool workspace_move_to_output(Con *ws, Output *output) {
     LOG("got output %p with content %p\n", output, content);
 
     Con *previously_visible_ws = TAILQ_FIRST(&(content->focus_head));
-    LOG("Previously visible workspace = %p / %s\n", previously_visible_ws, previously_visible_ws->name);
+    if (previously_visible_ws) {
+        DLOG("Previously visible workspace = %p / %s\n", previously_visible_ws, previously_visible_ws->name);
+    } else {
+        DLOG("No previously visible workspace on output.\n");
+    }
 
     bool workspace_was_visible = workspace_is_visible(ws);
     if (con_num_children(ws->parent) == 1) {
@@ -1024,6 +1028,10 @@ bool workspace_move_to_output(Con *ws, Output *output) {
         workspace_show(ws);
     }
 
+    if (!previously_visible_ws) {
+        return true;
+    }
+
     /* NB: We cannot simply work with previously_visible_ws since it might
      * have been cleaned up by workspace_show() already, depending on the
      * focus order/number of other workspaces on the output.