Fix and simplify sfrestore

Use a scratch buffer for each screen, this simplifies a lot allocations
and iterations, and allows restoring screens that contain more than one
frame.

3 files changed, 29 insertions, 47 deletions

diff --git a/src/actions.c b/src/actions.c
index 63f7245..1134631 100644
--- a/src/actions.c
+++ b/src/actions.c
@@ -5626,21 +5626,14 @@ cmdret *
 cmd_sfrestore (int interactively UNUSED, struct cmdarg **args)
 {
   char *copy, *ptr, *token;
-  int snum;
-  int out_of_screen = 0;
-  int s_count = screen_count ();
-
-  struct sf_data {
-    rp_screen *screen;
-    struct sbuf *frames;
-    int ret_restore;
-  } sf_data[s_count];
-
-  int sf_index = 0;
-  int i;
+  rp_screen *screen;
+  int out_of_screen = 0, restored = 0;
 
-  for (i = 0; i < s_count; i++)
-    sf_data[i].frames = sbuf_new (0);
+  list_for_each_entry (screen, &rp_screens, node)
+    {
+      sbuf_free (screen->scratch_buffer);
+      screen->scratch_buffer = sbuf_new (0);
+    }
 
   copy = xstrdup (ARG_STRING (0));
 
@@ -5653,7 +5646,7 @@ cmd_sfrestore (int interactively UNUSED, struct cmdarg **args)
 
   while (token != NULL)
     {
-      rp_screen *screen;
+      int snum;
 
       /* search for end of frameset */
       ptr = token;
@@ -5667,11 +5660,8 @@ cmd_sfrestore (int interactively UNUSED, struct cmdarg **args)
         {
           /* clobber screen number here, frestore() doesn't need it */
           *ptr = '\0';
-          sbuf_concat (sf_data[sf_index].frames, token);
-          sbuf_concat (sf_data[sf_index].frames, ",");
-
-          sf_data[sf_index].screen = screen;
-          sf_index++;
+          sbuf_concat (screen->scratch_buffer, token);
+          sbuf_concat (screen->scratch_buffer, ",");
         }
       else
         out_of_screen++;
@@ -5683,44 +5673,31 @@ cmd_sfrestore (int interactively UNUSED, struct cmdarg **args)
   free (copy);
 
   /* now restore the frames for each screen */
-  for (i = 0; i < sf_index; i++)
+  list_for_each_entry (screen, &rp_screens, node)
     {
-      cmdret * ret;
-      rp_screen *cur_screen;
-      struct sbuf *cur_frames;
-
-      cur_screen = sf_data[i].screen;
-      cur_frames = sf_data[i].frames;
+      cmdret *ret;
 
-      push_frame_undo (cur_screen); /* fdump to stack */
+      if (strlen (sbuf_get (screen->scratch_buffer)) == 0)
+        continue;
 
-      ret = frestore (sbuf_get (cur_frames), cur_screen);
-      sf_data[i].ret_restore = ret->success;
+      push_frame_undo (screen); /* fdump to stack */
 
+      /* XXX save the failure of each frestore and display it in case of error */
+      ret = frestore (sbuf_get (screen->scratch_buffer), screen);
+      if (ret->success)
+        restored++;
       cmdret_free (ret);
-    }
-
-  for (i = 0; i < s_count; i++)
-    sbuf_free (sf_data[i].frames);
 
-  for (i = 0; i < sf_index; i++)
-    {
-      int ret;
-
-      ret = sf_data[i].ret_restore;
-      if (ret != RET_SUCCESS)
-        {
-          return cmdret_new (ret, "Failed to restore frames for screen %d",
-                             sf_data[i].screen->number);
-        }
+      sbuf_free (screen->scratch_buffer);
+      screen->scratch_buffer = NULL;
     }
 
   if (!out_of_screen)
-    return cmdret_new (RET_SUCCESS, "Restored %i Frame(s)", sf_index);
+    return cmdret_new (RET_SUCCESS, "screens restored: %d", restored);
   else
     return cmdret_new (RET_SUCCESS,
-                       "Restored %i Frame(s), %i Frame(s) out of Screen(s)",
-                       sf_index, out_of_screen);
+                       "screens restored: %d, frames out of screen: %d",
+                       restored, out_of_screen);
 }
 
 cmdret *
diff --git a/src/data.h b/src/data.h
index 8019ec8..eb662ce 100644
--- a/src/data.h
+++ b/src/data.h
@@ -197,6 +197,9 @@ struct rp_screen
   /* This structure can exist in a list. */
   struct list_head node;
 
+  /* Used by sfrestore */
+  struct sbuf *scratch_buffer;
+
 #ifdef USE_XFT_FONT
   XftFont *xft_font;
   XftColor xft_fg_color, xft_bg_color;
diff --git a/src/screen.c b/src/screen.c
index 77ce41d..fedddfa 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -320,6 +320,8 @@ init_screen (rp_screen *s)
   /* Set the numset for the frames to our global numset. */
   s->frames_numset = rp_frame_numset;
 
+  s->scratch_buffer = NULL;
+
   /* Build the display string for each screen */
   buf = sbuf_new (0);
   sbuf_printf (buf, "DISPLAY=%s", DisplayString (dpy));