If I re-run cmake in a Unix build directory, it unconditionally
rewrites generated-games.h, which causes fuzzpuzz to be rebuilt. This
is a waste of effort in the extremely common case where the rewritten
generated-games.h is identical to the old one.
Now we write the data to a temporary file first, and use cmake's
'configure_file' command to copy that to generated-games.h, because it
so happens that configure_file checks if the two files are identical
and avoids updating the timestamp on the destination file if so.
(This will presumably also be a beneficial change on any other
platform that uses generated_games.h in the build, such as OS X. I
just hadn't noticed until it hit the build I most often re-run in an
existing build directory.)
cmake 3.21 has a more intuitively spelled command I could have used,
called 'file(COPY_FILE src dst ONLY_IF_DIFFERENT)'. But we currently
permit cmake all the way back to 3.5, so I can't use that.
When reporting that the game name in a save file isn't recognised,
don't include the name from the save file in the error message, partly
to avoid the complexity of freeing it properly on two different code
paths and partly because including unsanitized data from a
fuzzer-supplied save file in the error message just seems dangerous.
And properly sanitising it would waste the fuzzer's time exploring the
sanitising code.
Thanks to Ben Hutchings for reporting the bug.
Tracks allowed moves in execute_move() that shouldn't have been allowed,
like changing the state of the edges of the board. This moves couldn't
be generated by interpret_move(), but could be loaded from a save file.
Now execute_move() uses the same ui_can_flip_*() functions as
interpret_move() to decide whether a particular move is allowed. This
should prevent some assertion failures when loading corrupted save
files.
Towers' new_game() causes an assertion failure on game description
strings that contain spurious characters after a valid description, so
validate_desc() should also refuse such a description. The problem
could be demonstrated by editing the game description in the
"Specific" dialogue box to add a '!' at the end, which caused
"new_game: Assertion `!*p' failed.".
If a Mines save file contains a move after the player has already
died, this can lead to an assertion failure once there are more mines
that covered squares. Better to just reject any move after the
player's died.
It allowed V, W, X, Y, H, I, J, and K to appear in game descriptions
even though new_game() didn't ascribe any meaning to those letters and
would fail an assertion if they ever occurred. As far as I can tell,
those letters have never done anything, so I've just removed the
checks for them from validate_desc().
Without this, execute_move() can end up reading off the end of the
move string, which isn't very friendly. Also remove the comment
saying that the move string doesn't have to be null-terminated,
because now it does.
It can't signal an error, but it's worth documenting that it can
receive invalid input and should do what it can with it. I assume
that failing to decode game_ui data from a newer version generally
won't be disastrous the way failing to decode a description or move
string would be.
Previously if a move string starting with "M" contained anything else
other than a digit or a comma, execute_move() would spin trying to
parse it. Now it returns NULL.
Other games tolerate receiving an encoded game_ui even if they can
never generate one. This is sensible, since it means that if a new
version starts saving UI state, old versions can load save files
generated by those newer versions.
This makes sure that width * height <= INT_MAX, which it rather needs
to be. Also a similar check in decode_params when defaulting the
number of mines.
This makes sure that width * height <= INT_MAX, which it rather needs
to be. Also a similar check in decode_params when defaulting the
number of regions.
Every grid shape has its own limit, so this involved adding a new
interface between loopy.c and grid.c. The limits are based on ensuring
that the co-ordinate system of the grid doesn't overflow INT_MAX and
neither do the lengths of the face and dot arrays.
Though now I come to look at it I think the actual limits of grid.c are
much lower. Hmm.
Without this check, a corrupt save file can include a move that
removes an edge of the grid, and then is_solved() walks off the edge
of the grid causing a buffer over- or under-run.
To demonstrate the bug, load this save file in a build with
AddressSanitizer:
SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :8:Palisade
PARAMS :5:5x5n5
CPARAMS :5:5x5n5
DESC :0:
NSTATES :1:2
STATEPOS:1:2
MOVE :6:F0,0,1