ref: 8dd7ee300726872072075a5cdb35ebe9497e3adb
parent: 091fe57e0f8448e0972fa82dfac1f83f43147c5b
author: Simon Tatham <anakin@pobox.com>
date: Fri Jul 1 12:50:49 EDT 2005
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. [originally from svn r6045]
--- a/midend.c
+++ b/midend.c
@@ -855,6 +855,8 @@
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 @@
}
}
+ /*
+ * 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 @@
* 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 @@
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);