← 输入框|github.com|刷新
Skip to content

Report write failures in SaveToFile instead of claiming success - #1784

Draft
phkahler with Copilot wants to merge 2 commits into
masterfrom
copilot/fix-save-functionality-issue
Draft

Report write failures in SaveToFile instead of claiming success#1784
phkahler with Copilot wants to merge 2 commits into
masterfrom
copilot/fix-save-functionality-issue

Conversation

Copilot AI commented Oct 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

SaveToFile ignored ferror and the result of fclose, so a failed write (e.g. full disk) returned true. The target had already been truncated, so the user was told the save worked. In the GUI, GetFilenameAndSave then called RemoveAutosave(), deleting the only remaining copy.

  • Error detection (src/file.cpp)
    • After the last write, SaveToFile now checks ferror(fh) and the return value of fclose(fh).
    • On failure it shows the existing "Couldn't write to file" error and returns false.
    • fh is reset to NULL after closing.
  • Effect on callers
    • GetFilenameAndSave only removes the autosave when SaveToFile returns true, so the .slvs~ file now survives a failed save.
bool writeFailed = ferror(fh) != 0;
if(fclose(fh) != 0) writeFailed = true;
fh = NULL;
if(writeFailed) {
    Error("Couldn't write to file '%s'", filename.raw.c_str());
    return false;
}

Not addressed: the target is still opened with "wb", which truncates it before writing. A failed save can therefore still leave the .slvs truncated or empty. It is now reported, and the autosave is kept.

Making the save atomic (write a temp file in the same directory, then rename over the target) would close that gap. I left it out because src/platform/platform.h has no rename primitive, and std::rename fails on Windows when the destination exists. That would need a platform-layer addition and is better as a follow-up.

Copilot AI linked an issue Oct 5, 2026 that may be closed by this pull request
A failed save empties the sketch and still reports success #1783
Open
@phkahler
Apply remaining changes
a3b470a 
Co-authored-by: phkahler <14852918+phkahler@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix save functionality to prevent empty sketch on full disk Report write failures in SaveToFile instead of claiming success Oct 5, 2026
Copilot AI requested a review from phkahler October 5, 2026 19:44
@yottayoshida yottayoshida mentioned this pull request Oct 9, 2026
Merged
ayourk added a commit to ayourk/solvespace that referenced this pull request Oct 23, 2026
@ayourk
Merge upstream SolveSpace 3.3 and release the fork as 3.3hc
2cd2052 
Brings the fork up to upstream's v3.3 tag and lets it stand on its
own: a library that cannot be mistaken for stock libslvs, a version of
its own, and release builds for more platforms than upstream ships.

- Upstream 3.3: the headless CLI survives a missing linked file,
  constraint text can be selected while editing on Windows, fonts load
  from .ttc collections, a Brazilian Portuguese translation, sanitizer
  builds no longer need the gold linker, and solvespace#1748's solver test. Its
  damped step is not taken: ours (0031, the revision of solvespace#1748) stays,
  because upstream's still fails a case of solvespace#1105's sketch that ours
  solves. The debug tool keeps the fork's additions on top of
  upstream's.
- Ahead of upstream: three open pull requests. A save that fails to
  write now says so instead of claiming success (solvespace#1784); solvespace#1751's
  fallback for tangent line-surface intersections is refined (solvespace#1760);
  and solvespace#1787's tests of free parameters through substitution, a bug the
  fork had already fixed the same way, now guard that fix.
- Library: libslvshc, with pkg-config module slvshc and its header in
  its own directory, not libslvs. Its Slvs_System differs from stock,
  so a program built for one must never load the other, and the two
  install side by side. A program can now ask the library its version
  and check at startup that it matches the header it was built
  against.
- Version: 3.3hc, and 3.3hc.YYYY.MM.DD[.N] for dated builds; configure
  stops when the header's versions fall behind the project's.
- Releases: one workflow for nightlies and tagged releases, beside
  upstream's untouched ones. macOS DMGs per architecture with OpenMP
  linked in, static Windows executables for x86, x64 and arm64, AppImage
  and Flatpak for x86_64 and aarch64, the source, and the Python
  binding as one archive, never PyPI. Windows on Arm builds natively,
  telling two bundled libraries they are not on x86.
- README: where the fork diverges, ahead of upstream's own text, which
  is left unchanged so later merges do not conflict on it; the issue
  template sends everything else upstream.

Origin: upstream v3.3 (30e1c3a), upstream PRs solvespace#1784, solvespace#1760 and solvespace#1787
(tests), and HobbyCAD
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@phkahler phkahler Awaiting requested review from phkahler

Assignees

@phkahler phkahler

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

A failed save empties the sketch and still reports success

2 participants

@phkahler