Skip to content

fatfs: Free path copy when f_open fails (FATFS_FSTAT) - #5245

Open
herbenderbler wants to merge 545 commits into
prusa3d:masterfrom
herbenderbler:fix-failed-fopen-leak
Open

fatfs: Free path copy when f_open fails (FATFS_FSTAT)#5245
herbenderbler wants to merge 545 commits into
prusa3d:masterfrom
herbenderbler:fix-failed-fopen-leak

Conversation

@herbenderbler

@herbenderbler herbenderbler commented Apr 14, 2026

Copy link
Copy Markdown

Description

When FATFS_FSTAT is enabled, open_r allocates a heap copy of the opened path for fstat/fchmod. If f_open failed, that buffer was never freed because close_r does not run on a failed open. Each failed open leaked strlen(path) + 1 bytes until reboot. This change frees the path copy on the f_open error path and clears the pointer, matching cleanup in close_r.

Implementation Details

  • src/buddy/filesystem_fatfs.cpp: In open_r, inside if (result != FR_OK), add a FATFS_FSTAT-guarded block that calls free(f->path) when non-null and sets f->path to nullptr before existing FR_NO_FILESYSTEM handling and return -1.

Test/Documentation

  • To exercise the change: define FATFS_FSTAT, build a target that uses the FatFS devoptab, and open a path that makes f_open fail (e.g. missing file on USB); confirm heap does not grow on repeated attempts (or use a host-side Memcheck model of the same control flow).

bkerler and others added 30 commits August 14, 2025 08:49
subrepo:
  subdir:   "lib/Prusa-Error-Codes"
  merged:   "d64eb1056d"
upstream:
  origin:   "git@github.com:prusa3d/Prusa-Error-Codes.git"
  branch:   "master"
  commit:   "d64eb1056d"
git-subrepo:
  version:  "0.4.9"
  origin:   "???"
  commit:   "???"
As these happen fast & outside of the print, the motor may skip, but
that's OK. Don't report as filament stuck (because likely it isn't).

BFW-7545
And proceed with the test anyway.

BFW-7542.
Rework the ADC FS logic to get rid of the fixed FS span.

BFW-5338
Not using fs_span, get rid of it

BFW-5338
I will be adding REF_INS in the next commit

BFW-5338
It is no longer relevant to the config store.

The CalibrateInserted is now a bit mesy, but I intend to rework the whole calibration in the upcoming PR, so I would keep it this way for now.

BFW-5338
The selftest expects the fsensor to report HasFilament/NoFilament right after nins calibration is done, even though ins calibration is not yet done.

This forced me to reintroduce filament sensor span just for that situation.

Temporary solution till the new selftest comes.

BFW-5338
subrepo:
  subdir:   "lib/Prusa-Error-Codes"
  merged:   "a0138327b4"
upstream:
  origin:   "git@github.com:prusa3d/Prusa-Error-Codes.git"
  branch:   "master"
  commit:   "a0138327b4"
git-subrepo:
  version:  "0.4.9"
  origin:   "???"
  commit:   "???"
Add alignment_issue screen
Adjust finish screen
Edit texts
disable motors when appropriate

BFW-7413
vorner and others added 23 commits November 7, 2025 13:19
When running low on data in precise stepper or phase stepping, try to
wake up the move interrupt early, to refill.

* We hope there are some move segments cached to be processed even in
  case planner is currently blocking more of them.
* We hope there will be enough time or that the stall will be as short
  as possible if there isn't; certainly better situation than waiting
  for up to a whole 1ms.
* Remove the critical section from the planner side again - even if the
  moves are blocked, we may be able to do the refill from the cached
  data. Remove it with just an atomic flag so the move ISR will just
  skip looking there while it's being updated.

BFW-7936, BFW-7873.
With the running-dry wakeup, we can lower the queues again a bit - we
don't necessarily have to survive for a whole 1ms in that case.

BFW-7936, BFW-7873.
Instead of limiting the amount of segments processed to at most 1 per
1ms (or one call now), process as much as possible. We've observed
segments with shorter times than 1ms.

Even when we have the wakeup on running dry, it's better to keep the
buffers full and have smaller amount of the interrupt calls for
performance.

BFW-7873.
Instead of guessing if taking moves out of planner made progress
indirectly, just return it from the function. This is a slight
optimization of the move_isr loop.

BFW-7873.
Recent changes made it overflow from time to time (they either made the
stack of some interrupt function larger or the chance of multiple
interrupt stacking on each other got higher).

Trying to add 100 bytes (+alignment), as we don't have a direct way to
measure how much we need.

BFW-7873.
Cherry-pick with added C1 values
Co-Authored-By: Dano Pernis <danopernis@gmail.com>
Co-Authored-By: Ondřej Veselý <o.vesely42@gmail.com>

BFW-7395
BFW-8021
More granular tracking of the printer's setup progress. And display only the ones missing when printer si selectively factory reset.

BFW-7867
Crash recovery was hard-switching to Resuming_Begin print state, which:
- Did not trigger media_prefetch_call()
- Skipped the resuming buffering phase.

Using print_resume unifies the functionality and print_resume makes sure that all necessary steps are done.

BFW-8082
@herbenderbler
herbenderbler force-pushed the fix-failed-fopen-leak branch 2 times, most recently from acf354f to 93eb8b6 Compare April 14, 2026 14:38
When FATFS_FSTAT is enabled, open_r allocates a path copy for fstat/fchmod.
If f_open fails, close_r never runs, so the buffer was leaked. Free f->path
on the error path and clear the pointer, matching close_r.
@herbenderbler
herbenderbler force-pushed the fix-failed-fopen-leak branch from 93eb8b6 to 1d4cdc6 Compare April 14, 2026 14:38
@herbenderbler herbenderbler changed the title fatfs: free path copy when f_open fails (FATFS_FSTAT) fatfs: Free path copy when f_open fails (FATFS_FSTAT) Apr 14, 2026
@herbenderbler
herbenderbler marked this pull request as ready for review April 14, 2026 14:40
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.

8 participants