Skip to content

filemax: Fix crash when undoing a page deletion - #88

Open
sjg20 wants to merge 1 commit into
masterfrom
fix-restore-pages-crash
Open

filemax: Fix crash when undoing a page deletion#88
sjg20 wants to merge 1 commit into
masterfrom
fix-restore-pages-crash

Conversation

@sjg20

@sjg20 sjg20 commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Fix segfault when undoing a page deletion

Deleting pages from a stack (untick the green "keep" arrows) and then
pressing Ctrl-Z to undo crashes with a segfault:

Thread 1 received signal SIGSEGV
#0  __GI_fseek (fp=0x0, ...) at libio/fseek.c:35
#1  Filemax::max_read_data (...) at filemax.cpp:801
#2  Filemax::read_chunk_buf (...) at filemax.cpp:2010
#3  Filemax::restore_chunknum (...)

Root cause

restorePages() calls restore_page(), which reads each deleted page's
chunk data back from disc via max_read_data(). Unlike the matching
removePages() — which wraps free_page() in ensure_open() /
ensure_closed()restorePages() never opens the file first. _fin
is NULL between operations, so fseek(NULL, …) crashes.

Fix

  • Open the file around restore_page() and close it afterwards,
    mirroring removePages().
  • Defensively make max_read_data() / max_write_data() return an
    error instead of calling fseek() on a NULL stream, so a similar
    omission can't crash again.
  • Drop a stray debug printf() that fired on every restored page.

Test

Adds TestFile::testRemoveRestorePages: removes a page then restores
it, checking the page count round-trips and the restored page still
decodes (this is the path that used to crash).

Deleting pages from a stack and then pressing Ctrl-Z to undo crashes with
a segfault in fseek() with a NULL stream. restorePages() calls
restore_page(), which reads each deleted page's chunk data back from
disc through max_read_data(), but unlike removePages() it never opens
the file first. The stream _fin is left NULL between operations, so the
read dereferences a NULL FILE *.

Open the file around restore_page(), mirroring how removePages() opens
it around free_page(), and close it again afterwards. As a safety net,
make max_read_data() and max_write_data() return an error instead of
calling fseek() on a NULL stream, so a similar mistake can no longer
crash the program. Also drop a stray debug printf() that fired on every
restored page.

Add a Filemax test that removes a page and then restores it, checking
the page count round-trips and the restored page still decodes.

Co-developed-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant