From 5c9f61fd500f6fd53a9f33721a1e66b9d5e1d9cc Mon Sep 17 00:00:00 2001
From: simon <simon@cda61777-01e9-0310-a592-d414129be87e>
Date: Fri, 1 Jul 2005 16:50:49 +0000
Subject: [PATCH] James Harvey points out that entering an invalid game ID can
 affect the current midend state even if you don't subsequently enter a valid
 one. Reorganise midend_game_id_int() so that (just like midend_deserialise())
 it does all its error checking before altering anything in the midend's
 persistent data, so that it either succeeds completely or fails before doing
 anything at all.

git-svn-id: svn://svn.tartarus.org/sgt/puzzles@6045 cda61777-01e9-0310-a592-d414129be87e
---
 midend.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 54 insertions(+), 16 deletions(-)

diff --git a/midend.c b/midend.c
index a764c77..7bc2d4f 100644
--- a/midend.c
+++ b/midend.c
@@ -855,6 +855,8 @@ config_item *midend_get_config(midend_data *me, int which, char **wintitle)
 static char *midend_game_id_int(midend_data *me, char *id, int defmode)
 {
     char *error, *par, *desc, *seed;
+    game_params *newcurparams, *newparams, *oldparams1, *oldparams2;
+    int free_params;
 
     seed = strchr(id, '#');
     desc = strchr(id, ':');
@@ -894,18 +896,24 @@ static char *midend_game_id_int(midend_data *me, char *id, int defmode)
         }
     }
 
+    /*
+     * We must be reasonably careful here not to modify anything in
+     * `me' until we have finished validating things. This function
+     * must either return an error and do nothing to the midend, or
+     * return success and do everything; nothing in between is
+     * acceptable.
+     */
+    newcurparams = newparams = oldparams1 = oldparams2 = NULL;
+
     if (par) {
-        game_params *tmpparams;
-        tmpparams = me->ourgame->dup_params(me->params);
-        me->ourgame->decode_params(tmpparams, par);
-        error = me->ourgame->validate_params(tmpparams);
+        newcurparams = me->ourgame->dup_params(me->params);
+        me->ourgame->decode_params(newcurparams, par);
+        error = me->ourgame->validate_params(newcurparams);
         if (error) {
-            me->ourgame->free_params(tmpparams);
+            me->ourgame->free_params(newcurparams);
             return error;
         }
-        if (me->curparams)
-            me->ourgame->free_params(me->curparams);
-        me->curparams = tmpparams;
+        oldparams1 = me->curparams;
 
         /*
          * Now filter only the persistent parts of this state into
@@ -913,16 +921,50 @@ static char *midend_game_id_int(midend_data *me, char *id, int defmode)
          * received a params string in which case the whole lot is
          * persistent.
          */
+        oldparams2 = me->params;
         if (seed || desc) {
-            char *tmpstr = me->ourgame->encode_params(tmpparams, FALSE);
-            me->ourgame->decode_params(me->params, tmpstr);
+            char *tmpstr;
+
+            newparams = me->ourgame->dup_params(me->params);
+
+            tmpstr = me->ourgame->encode_params(newcurparams, FALSE);
+            me->ourgame->decode_params(newparams, tmpstr);
+
             sfree(tmpstr);
         } else {
-            me->ourgame->free_params(me->params);
-            me->params = me->ourgame->dup_params(tmpparams);
+            newparams = me->ourgame->dup_params(newcurparams);
         }
+        free_params = TRUE;
+    } else {
+        newcurparams = me->curparams;
+        newparams = me->params;
+        free_params = FALSE;
     }
 
+    if (desc) {
+        error = me->ourgame->validate_desc(newparams, desc);
+        if (error) {
+            if (free_params) {
+                if (newcurparams)
+                    me->ourgame->free_params(newcurparams);
+                if (newparams)
+                    me->ourgame->free_params(newparams);
+            }
+            return error;
+        }
+    }
+
+    /*
+     * Now we've got past all possible error points. Update the
+     * midend itself.
+     */
+    me->params = newparams;
+    me->curparams = newcurparams;
+    if (oldparams1)
+        me->ourgame->free_params(oldparams1);
+    if (oldparams2)
+        me->ourgame->free_params(oldparams2);
+
     sfree(me->desc);
     sfree(me->privdesc);
     me->desc = me->privdesc = NULL;
@@ -930,10 +972,6 @@ static char *midend_game_id_int(midend_data *me, char *id, int defmode)
     me->seedstr = NULL;
 
     if (desc) {
-        error = me->ourgame->validate_desc(me->params, desc);
-        if (error)
-            return error;
-
         me->desc = dupstr(desc);
         me->genmode = GOT_DESC;
         sfree(me->aux_info);
-- 
2.11.0