aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* js: Add a trivial error handler that alert()sBen Harris2023-02-19
| | | | | | | I'm not quite sure how useful it will be, but it does at least catch an assertion failure in main() and present an opaque message in a box, which is better than stopping and putting a message in the console where no-one will see it.
* Convert a lot of floating-point constants to single precisionBen Harris2023-02-19
| | | | | | | | | | | | | | | | | | For reasons now lost to history, Puzzles generally uses single-precision floating point. However, C floating-point constants are by default double-precision, and if they're then operated on along with a single-precision variable the value of the variable gets promoted to double precision, then the operation gets done, and then often the result gets converted back to single precision again. This is obviously silly, so I've used Clang's "-Wdouble-promotion" to find instances of this and mark the constants as single-precision as well. This is a bit awkward for PI, which ends up with a cast. Maybe there should be a PIF, or maybe PI should just be single-precision. This doesn't eliminate all warnings from -Wdouble-promotion. Some of the others might merit fixing but adding explicit casts to double just to shut the compiler up would be going too far, I feel.
* Miscellaneous const fixesBen Harris2023-02-18
| | | | | These are cases where -Wcast-qual complained and the only change needed was to add or remove a "const" (or sometimes an entire cast).
* Use unreserved macro names for multiple-include protectionBen Harris2023-02-18
| | | | | | | Some headers used macros named like _THING_H for multiple-include protection. That style of name is reserved in ISO C, though, so I've replaced it with PUZZLES_THING_H which is my favourite of the other styles in use.
* Replace a buch of "const static" with "static const"Ben Harris2023-02-18
| | | | C18 section 6.11.5 says that "const static" is obsolescent.
* Unequal: use %u to format an unsigned intBen Harris2023-02-18
|
* Undead: be a bit more careful about sprintf buffer sizesBen Harris2023-02-18
|
* Revert "Stop persistent-mode fuzzpuzz exiting prematurely"Ben Harris2023-02-18
| | | | | | | That was completely wrong: a "continue" at the end of the loop is unnecessary. This reverts commit b91f9824b6f73290051025317f3387c7212fa05f.
* Make emcc.c clean under -Wmissing-prototypes etcBen Harris2023-02-18
| | | | | | | | Also -Wstrict-prototypes and -Wmissing-variable-declarations. Functions that are called from JavaScript now have a separate declaration at the top of the file with a comment reminding one to update emcclib.js if they're changed.
* Mosaic: ignore taps above/left of the gridChris Boyle2023-02-18
| | | | | | | Thanks to Larry Hastings for the patch (tweaked to include drag/release). (cherry picked from Android port, commit 377e61b144c518a3d9efba66be08bf00ff6596e8)
* Stop persistent-mode fuzzpuzz exiting prematurelyBen Harris2023-02-18
| | | | In the transition to fuzz_one() I'd lost a "continue".
* Support Honggfuzz's persistent mode in fuzzpuzzBen Harris2023-02-18
| | | | | | | | Unlike AFL, Honggfuzz's compiler wrapper doesn't provide a convenient preprocessor macro, so we have to have CMake detect the existence of HF_ITER. Also the resulting program can't run outside of Honggfuzz, so maybe some additional cleverness is called for there as well. Still, it makes Honggfuzz go ten times faster, which is nice.
* Use -Wmissing-prototypes with GCC as wellBen Harris2023-02-18
| | | | | -Wmissing-prototypes was what I wanted all along, but somehow I'd mis-read the documentation and thought it wasn't.
* Buildscr: include a test build with clang + STRICT.Simon Tatham2023-02-18
| | | | | | I've just enabled a warning that only fires in that mode, so we need to keep running the build in that configuration to ensure further instances of the warning aren't introduced.
* Fix missing statics and #includes on variables.Simon Tatham2023-02-18
| | | | | | | | | | | | | | | | | | | | | | | After Ben fixed all the unwanted global functions by using gcc's -Wmissing-declarations to spot any that were not predeclared, I remembered that clang has -Wmissing-variable-declarations, which does the same job for global objects. Enabled it in -DSTRICT=ON, and made the code clean under it. Mostly this was just a matter of sticking 'static' on the front of things. One variable was outright removed ('verbose' in signpost.c) because after I made it static clang was then able to spot that it was also unused. The more interesting cases were the ones where declarations had to be _added_ to header files. In particular, in COMBINED builds, puzzles.h now arranges to have predeclared each 'game' structure defined by a puzzle backend. Also there's a new tiny header file gtk.h, containing the declarations of xpm_icons and n_xpm_icons which are exported by each puzzle's autogenerated icon source file and by no-icon.c. Happily even the real XPM icon files were generated by our own Perl script rather than being raw xpm output from ImageMagick, so there was no difficulty adding the corresponding #include in there.
* Fix unused variable warnings from clang.Simon Tatham2023-02-18
| | | | | | | | If you enable -DSTRICT=ON in cmake and also build with clang, it reports a couple of variables set but not otherwise used. One was genuinely unused ('loop_found' in loop_deductions in Loopy); the other is used by debug statements that are usually but not always compiled out.
* Add -Wmissing-prototypes to STRICT clang builds.Simon Tatham2023-02-18
| | | | | | | | Ben added -Wmissing-declarations in commit 3a3e491a8bc624e for gcc builds, and observed that clang's option of the same name doesn't do the same job. But clang does _have_ an option to do the same job: it's just spelled differently. Added -Wmissing-prototypes in clang builds, so that those will check the same thing.
* Enable -Wmissing-declarations in STRICT mode on GCCBen Harris2023-02-18
| | | | | | | Clang's -Wmissing-declarations means something quite different, so we only enable it on GCC. This is slightly silly since Clang's -Wmissing-declarations is enabled by default, but it makes it clear that we know they're different.
* Mark many more function (and some objects) staticBen Harris2023-02-18
| | | | | | | | | | | | | I noticed commit db3b531e2cab765a00475054d2e9046c9d0437d3 in the history where Simon added a bunch of "static" qualifiers. That suggested that consistently marking internal functions "static" is desirable, so I tried a build using GCC's -Wmissing-declarations, which requires prior declaration (presumed to be in a header file) of all global functions. This commit makes the GTK build clean under GCC's -Wmissing-declarations. I've also adding "static" to a few obviously internal objects, but GCC doesn't complain about those so I certainly haven't got them all.
* Call deallocate() in matching.c test routinesBen Harris2023-02-18
| | | | | | | This is mostly so that the function is used at all, but I've also removed another memory leak from --autotest mode to make it apparently leak-free. The testing from standard input mode has more leaks than I want to fix.
* Adjust fuzzpuzz sample shell commands to not include "/*"Ben Harris2023-02-16
| | | | | | GCC warns about that character sequence in a comment. I shouldn't have assumed that having only edited a comment meant I could get away without a test build.
* Tracks: set drag_s{x,y} even if starting off-gridChris Boyle2023-02-16
| | | | | | | | | | | Otherwise, if subsequent mouse/finger movement lines up with the previous drag attempt's start, then suddenly a drag is in progress from there, which is confusing. Fixes #588 (cherry picked from Android port, commit 8ce1bbe460d70a915caf2dbeb30354d22dc8a8ef)
* Update and expand comment at the head of fuzzpuzzBen Harris2023-02-16
| | | | | It now correctly describes what fuzzpuzz does. It also provides an example of how to use it with AFL++.
* Separate fuzzing and harness in fuzzpuzzBen Harris2023-02-16
| | | | | | | | | There's now a function, fuzz_one(), that processes a single save file, and main() arranges to call this a suitable number of times depending on whether we're in AFL persistent mode or not. This makes things a bit cleaner, and will probably make adding good support for other fuzzers, or just switching entirely to the horrible but popular libFuzzer interface, simpler.
* js: Hide type menu if there's only one preset and no configurationBen Harris2023-02-16
| | | | It seems a bit silly to display it when there's only one option.
* Solo: cope with pencil marks when tilesize == 1Ben Harris2023-02-16
| | | | | | | | | | | | Solo's layout calculations for pencil marks could fail with a tilesize of 1, generating an assertion failure: "draw_number: Assertion `pbest > 0' failed." This was reported as Debian bug #905852. My solution is slightly silly, namely to change a ">" in the test for whether a new layout is the best so far to ">=". This allows for finding a (terrible) layout even for tilesize == 1, and also has the side-effect of slightly preserring wide layouts over tall ones. Personally, I think that's an improvement.
* Note in the documentation that Pattern clues are in orderBen Harris2023-02-15
| | | | Requested in Debian bug #642207 and seems perfectly reasonable.
* Tighten grid-size limit in MinesBen Harris2023-02-15
| | | | | | | | | | | | | | | | | | | | | | Mines uses random_upto() to decide where to place mines, and random_upto() takes a maximum limit of 2^28-1, so limit the number of grid squares to that (or INT_MAX if someone's still trying to build on a 16-bit system). This avoids an assertion failure: "random_upto: Assertion `bits < 32' failed." which can be demonstrated by this save file: SAVEFILE:41:Simon Tatham's Portable Puzzle Collection VERSION :1:1 GAME :5:Mines PARAMS :5:18090 CPARAMS :5:18090 DESC :11:r9,u,MEdff6 UI :2:D0 TIME :1:0 NSTATES :1:2 STATEPOS:1:2 MOVE :4:O2,1
* Make sure that moves in Flood use only valid coloursBen Harris2023-02-14
| | | | | | | | | | | | | | | | | | | If execute_move() receieves a move that uses a colour beyond the range for the current game, it now rejects it. Without this a solve string containing an invalid colour would cause an assertion failure: "fill: Assertion `oldcolour != newcolour' failed." While I was in the area I put a range check on colours for normal moves as well. To demonstrate the problem, load this save file: SAVEFILE:41:Simon Tatham's Portable Puzzle Collection VERSION :1:1 GAME :5:Flood PARAMS :7:6x6c6m5 CPARAMS :7:6x6c6m3 DESC :39:432242034203340350204502505323231342,17 NSTATES :1:2 STATEPOS:1:2 MOVE :2:S6
* Fix over-long lines in devel.butBen Harris2023-02-13
| | | | Most of these are my fault anyway.
* More validation of solve moves in FloodBen Harris2023-02-13
| | | | | | | | To avoid assertion failures while painting it, we need to ensure that the purported solution in a solve move doesn't include filling with the current top-left colour at any point. That means checking the first entry against the current top-left colours, and each later one against its predecessor.
* Validate that save file values are ASCII (mostly)Ben Harris2023-02-13
| | | | | | | | | | | | | | Apart from "SEED" records, all values in save files generated by Puzzles should be printable ASCII. This is enforced by assertion in the saving code. However, if a save file with non-ASCII move strings (for instance) manages to get loaded then these non-ASCII values can cause an assertion failure on saving. Instead, the loading code now checks values for ASCIIness. This will not only avoid problems when re-saving files, but will also defend the various internal parsers from at least some evil strings. It shouldn't invalidate any save files actually generated by Puzzles, but it will sadly invalidate some of my fuzzing corpus.
* Extend fuzzpuzz to test more codeBen Harris2023-02-13
| | | | | | Now if the input save file loads correctly, fuzzpuzz asks the back-end to draw the puzzle. All the drawing operations are no-ops, but this tests the drawing code in the back-end.
* Reserialise the game in fuzzpuzzBen Harris2023-02-13
| | | | | This means that the serialising code gets tested, and also provides a convenient way to canonicalise a (valid) save file.
* Avoid division by zero in Cube grid-size checksBen Harris2023-02-13
| | | | | | | | On a triangular grid, Cube allows either d1 or d2 (but not both) to be zero, so it's important to check that each one is not zero before dividing by it. The crash could be triggered by, for instance "cube t0x2".
* Mosaic: don't duplicate the description being validatedBen Harris2023-02-13
| | | | | | Mosaic's validate_desc() doesn't write to the description string, so it has no need to make a copy of it. And if it doesn't copy it, it can't leak the copy.
* Loopy: free the grid description string if it's invalidBen Harris2023-02-13
|
* Twiddle: don't read off the end of parameter strings ending 'm'Ben Harris2023-02-13
| | | | | The overrun could be demonstrated by specifying a parameter string of "3x3m" to a build with AddressSanitizer.
* Free new game_state properly in Mosaic's execute_move()Ben Harris2023-02-13
| | | | | | Using sfree() rather than free_game() in the error paths meant that various arrays referenced from the game_state weren't properly freed. Also one error path didn't free the game_state at all.
* Remember to free the numcolours array from Pattern's drawstateBen Harris2023-02-13
|
* Don't leak duplicate edges in UntangleBen Harris2023-02-13
| | | | | | | Untangle game descriptions are allowed to contain duplicate edges, and add234() can handle deduping them. However, when add234() reports that your newly-allocated edge is a duplicate, it's important to free it again.
* Undead: check the return value of sscanf() in execute_move()Ben Harris2023-02-13
| | | | | | | | | | sscanf() assigns its output in order, so if a conversion specifier fails to match, a later "%n" specifier will also not get its result assigned. In Undead's execute_move(), this led to the result of "%n" being used without being initialised. That could cause it to try to parse arbitrary memory as part of the move string, which shouldn't be a security problem (since execute_move() handles untrusted input anyway), but could lead to a crash and certainly wasn't helpful.
* Remember to free the to_draw member from Net's drawstateBen Harris2023-02-13
|
* Don't leak grids in Loopy's validate_desc()Ben Harris2023-02-13
|
* Remember to free the actual_board array in MosaicBen Harris2023-02-13
|
* Fix memory leaks in Keen's validate_desc()Ben Harris2023-02-13
| | | | | Keen uses a DSF to validate its game descriptions and almost always failed to free it, even when the validation succeeded.
* Allow more general cross-shaped boards in PegsBen Harris2023-02-12
| | | | | | | I found a plausible looking Web page claiming that various different sizes of cross are soluble, and some of them are quite widespread. I've enabled the ones that are symmetric enough that the existing game generator can lay them out.
* Don't allow moves that change the constraints in UnequalBen Harris2023-02-11
| | | | | | | | | | | | | | | | | | | | | | | Unequal has a flags word per cell. Some of those flags are fixed, like the locations of the ">" signs, but others indicate errors and are used to allow the player to mark clues as "spent". Move strings beginning with "F" allow the user to change the "spent" flags, but they shouldn't allow the user to change any other flags, especially those marking the constraints. Without this fix, the following save file gives a "solver_nminmax: Assertion `x >= 0 && y >= 0 && x < o && y < o' failed" after it adds a clue that points off the board: SAVEFILE:41:Simon Tatham's Portable Puzzle Collection GAME :7:Unequal PARAMS :3:3e0 CPARAMS :3:3e0 DESC :17:0,0,0,0,0,0,0,0,0 NSTATES :2:3 STATEPOS:1:3 MOVE :6:F2,0,4 MOVE :1:H
* Cleanly reject more ill-formed solve moves in FloodBen Harris2023-02-11
| | | | | | | | | | | | | The fix in e4112b3 was incomplete: there was another assertion that could be failed by a save file with an ill-formed solve move. That now gets rejected properly. Here's an example save file to demonstrate the problem: SAVEFILE:41:Simon Tatham's Portable Puzzle Collection GAME :5:Flood PARAMS :7:6x6c6m0 CPARAMS :7:6x6c6m0 DESC :39:000000000000000000000000000000000000,00 NSTATES :1:2 STATEPOS:1:2 MOVE :1:S
* Check state is valid at the end of a move in PearlBen Harris2023-02-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A Pearl move string contains a sequence of sub-moves, each of which can affect the state of the connection between the centre of a square and one of its edges. interpret_move() generates these in pairs so that the two halves of a connection between the centres of adjacent squares stay in the same state. If, however, a save file contains mismatched half-moves, execute_move() should ideally return NULL rather than causing an assertion failure. This has to be checked at the end of the whole move string, so I've arranged for check_completion() to return a boolean indicating whether the current state (and hence the move preceding it) is valid. It now returns 'false' when a connection stops at a square boundary or when it goes off the board. These conditions used to be assertion failures, and now they just cause the move to be rejected. This supersedes the check for off-board connections added in 15f4fa8, since now check_completion() can check for off-board links for the whole board at once. This save file trivially demonstrates the problem, causing "dsf_update_completion: Assertion `state->lines[bc] & F(dir)' failed" without this fix: SAVEFILE:41:Simon Tatham's Portable Puzzle Collection GAME :5:Pearl PARAMS :5:6x6t0 CPARAMS :5:6x6t0 DESC :17:BbBfWceBbWaBWWgWB NSTATES :1:2 STATEPOS:1:2 MOVE :6:R1,0,0
generated by cgit v1.1 at 2026-05-13 14:02:54 +0000