Skip to content

extmod/vfs: Skip littlefs detection if mp_vfs_blockdev_read_ext() fails.#19306

Open
projectgus wants to merge 1 commit into
micropython:masterfrom
projectgus:bugfix/vfs_uninit_buf
Open

extmod/vfs: Skip littlefs detection if mp_vfs_blockdev_read_ext() fails.#19306
projectgus wants to merge 1 commit into
micropython:masterfrom
projectgus:bugfix/vfs_uninit_buf

Conversation

@projectgus
Copy link
Copy Markdown
Contributor

@projectgus projectgus commented Jun 4, 2026 •
edited
Loading

Summary

I noticed in passing that buf can be read uninitialised while checking for littlefs, if mp_vfs_blockdev_read_ext fails.

Rather than initialise buf (expensive), have opted to check the return value of the read function.

This work was funded through GitHub Sponsors.

Testing

  • Flashed a TEENSY41 and an RPI_PICO board, verified the littlefs filesystem still mounted on boot.

Trade-offs and Alternatives

  • Could ignore this, as the chances of this function failing are very low and then the chances of the string littlefs2 ending up on the stack independently of the internal flash is additionally low (and the first match will return, so a stale copy should never end up being checked on the second iteration.)
  • Could zero buf, but as mentioned above, this is more expensive than checking the return value.
  • Small chance of regression if there's a port block driver which doesn't return 0 here for success, but as the littlefs2 read function also calls mp_vfs_blockdev_read_ext and returns the result to littlefs2 it seems like such a port block driver would already not work with littlefs.

Generative AI

I did not use generative AI tools when creating this PR.

@projectgus
extmod/vfs: Skip littlefs detection if mp_vfs_blockdev_read_ext() fails.
a9d7a75 
Otherwise 'buf' is read uninitialised.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
Signed-off-by: Angus Gratton <angus@redyak.com.au>
@projectgus projectgus added the extmod Relates to extmod/ directory in source label Jun 4, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 4, 2026

Code size report:

Reference:  esp8266/network_wlan: Allow bytes() objects as MAC address. [be9a819]
Comparison: extmod/vfs: Skip littlefs detection if mp_vfs_blockdev_read_ext() fails. [merge of a9d7a75]
  mpy-cross:    +0 +0.000% 
   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:   +16 +0.002% standard
      stm32:   -16 -0.004% PYBV10
      esp32:   -40 -0.002% ESP32_GENERIC
     mimxrt:   -16 -0.004% TEENSY40
        rp2:   -16 -0.002% RPI_PICO_W
       samd:   -16 -0.006% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 4, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.47%. Comparing base (be9a819) to head (a9d7a75).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
extmod/vfs.c 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #19306      +/-   ##
==========================================
- Coverage   98.47%   98.47%   -0.01%     
==========================================
  Files         176      176              
  Lines       22845    22845              
==========================================
- Hits        22497    22496       -1     
- Misses        348      349       +1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dpgeorge
Copy link
Copy Markdown
Member

dpgeorge commented Jun 4, 2026

This looks very reasonable to me, to check the return value (regardless of anything else, that seems like the right thing to do).

Small chance of regression if there's a port block driver which doesn't return 0 here for success, but as the littlefs2 read function also calls mp_vfs_blockdev_read_ext and returns the result to littlefs2 it seems like such a port block driver would already not work with littlefs.

I very much doubt there's such a driver that this change would break. So I think it's OK.

But I do see that code coverage is failing. Is there a way to test this change? Maybe try to auto-mount a block device that fails on all reads?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

extmod Relates to extmod/ directory in source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@projectgus @dpgeorge