Style tweaks to the newgame_undo patch.

I've renamed the new midend variables to match my usual naming
convention of using 'size' for the total buffer size and 'len' for the
amount of currently used space (and a couple of other variables to
match those in turn), partly for consistency and also because the name
'midend_undo_used' made me half-expect it to be a boolean. The buffer
itself is called 'midend_undo_buf', again to avoid making it sound
like a function or flag.

Buffer growth is still geometric but less aggressive (factor of 5/4
each time instead of 2), in the hope of wasting less memory on low-RAM
platforms; newgame_undo_deserialise_read should have been static, and
now is; newgame_undo_buf is initialised using NULL rather than 0 so it
doesn't look like an integer, and is freed with the rest of the
midend.

And I think we _should_ enforce by assertion that midend_deserialise
didn't return an error, because there's no reason it ever should in
this situation (unlike when reading from a file, where the user might
have provided the wrong file or a corrupted one). This immediately
allowed me to spot a bug in the existing deserialisation code :-)

diff --git a/midend.c b/midend.c
index 03d6e98f8e0f10c88915f409be0fa3ea2685d0bf..5dd896f5b66fb9a162c186660e1248754eb69133 100644 (file)
--- a/midend.c
+++ b/midend.c
@@ -63,8 +63,8 @@ struct midend {
     int nstates, statesize, statepos;
     struct midend_state_entry *states;
 
-    void *newgame_undo;
-    int newgame_undo_avail, newgame_undo_used;
+    void *newgame_undo_buf;
+    int newgame_undo_len, newgame_undo_size;
 
     game_params *params, *curparams;
     game_drawstate *drawstate;
@@ -158,8 +158,8 @@ midend *midend_new(frontend *fe, const game *ourgame,
     me->random = random_new(randseed, randseedsize);
     me->nstates = me->statesize = me->statepos = 0;
     me->states = NULL;
-    me->newgame_undo = 0;
-    me->newgame_undo_avail = me->newgame_undo_used = 0;
+    me->newgame_undo_buf = NULL;
+    me->newgame_undo_size = me->newgame_undo_len = 0;
     me->params = ourgame->default_params();
     me->game_id_change_notify_function = NULL;
     me->game_id_change_notify_ctx = NULL;
@@ -257,6 +257,7 @@ void midend_free(midend *me)
     if (me->drawing)
        drawing_free(me->drawing);
     random_free(me->random);
+    sfree(me->newgame_undo_buf);
     sfree(me->states);
     sfree(me->desc);
     sfree(me->privdesc);
@@ -386,23 +387,22 @@ void midend_force_redraw(midend *me)
 static void newgame_serialise_write(void *ctx, void *buf, int len)
 {
     midend *const me = ctx;
-    int new_used;
-
-    assert(len < INT_MAX - me->newgame_undo_used);
-    new_used = me->newgame_undo_used + len;
-    if (new_used > me-> newgame_undo_avail) {
-       me->newgame_undo_avail = max(me->newgame_undo_avail, new_used);
-       me->newgame_undo_avail *= 2;
-       me->newgame_undo = sresize(me->newgame_undo,
-                                  me->newgame_undo_avail, char);
+    int new_len;
+
+    assert(len < INT_MAX - me->newgame_undo_len);
+    new_len = me->newgame_undo_len + len;
+    if (new_len > me->newgame_undo_size) {
+       me->newgame_undo_size = new_len + new_len / 4 + 1024;
+       me->newgame_undo_buf = sresize(me->newgame_undo_buf,
+                                       me->newgame_undo_size, char);
     }
-    memcpy(me->newgame_undo + me->newgame_undo_used, buf, len);
-    me->newgame_undo_used = new_used;
+    memcpy(me->newgame_undo_buf + me->newgame_undo_len, buf, len);
+    me->newgame_undo_len = new_len;
 }
 
 void midend_new_game(midend *me)
 {
-    me->newgame_undo_used = 0;
+    me->newgame_undo_len = 0;
     midend_serialise(me, newgame_serialise_write, me);
 
     midend_stop_anim(me);
@@ -518,7 +518,7 @@ void midend_new_game(midend *me)
 
 int midend_can_undo(midend *me)
 {
-    return (me->statepos > 1 || me->newgame_undo_used);
+    return (me->statepos > 1 || me->newgame_undo_len);
 }
 
 int midend_can_redo(midend *me)
@@ -528,16 +528,16 @@ int midend_can_redo(midend *me)
 
 struct newgame_undo_deserialise_read_ctx {
     midend *me;
-    int size, pos;
+    int len, pos;
 };
 
-int newgame_undo_deserialise_read(void *ctx, void *buf, int len)
+static int newgame_undo_deserialise_read(void *ctx, void *buf, int len)
 {
     struct newgame_undo_deserialise_read_ctx *const rctx = ctx;
     midend *const me = rctx->me;
 
-    int use = min(len, rctx->size - rctx->pos);
-    memcpy(buf, me->newgame_undo + rctx->pos, use);
+    int use = min(len, rctx->len - rctx->pos);
+    memcpy(buf, me->newgame_undo_buf + rctx->pos, use);
     rctx->pos += use;
     return use;
 }
@@ -554,17 +554,15 @@ static int midend_undo(midend *me)
        me->statepos--;
         me->dir = -1;
         return 1;
-    } else if (me->newgame_undo_used) {
+    } else if (me->newgame_undo_len) {
        /* This undo cannot be undone with redo */
        struct newgame_undo_deserialise_read_ctx rctx;
        rctx.me = me;
-       rctx.size = me->newgame_undo_used; /* copy for reentrancy safety */
+       rctx.len = me->newgame_undo_len; /* copy for reentrancy safety */
        rctx.pos = 0;
         deserialise_error =
            midend_deserialise(me, newgame_undo_deserialise_read, &rctx);
-       if (deserialise_error)
-           /* umm, better to carry on than to crash ? */
-           return 0;
+       assert(!deserialise_error);
        return 1;
     } else
         return 0;
@@ -2130,7 +2128,7 @@ static char *midend_deserialise_internal(
      * more sophisticated way to decide when to discard the previous
      * game state.
      */
-    me->newgame_undo_used = 0;
+    me->newgame_undo_len = 0;
 
     {
         game_params *tmp;