search

littlefs-project / littlefs

A little fail-safe filesystem designed for microcontrollers
BSD 3-Clause "New" or "Revised" License
4.92k stars 774 forks source link

lfs_fs_raw* functions should be static #884

Closed DvdGiessen closed 8 months ago

DvdGiessen commented 8 months ago

Two new functions introduced in 2.6.0 (lfs_fs_mkconsistent, 259535ee73a792556bb16eebfbd076f467c5bdfa) and 2.8.0 (lfs_fs_grow, 23505fa9fa4f2d863f046c0bfad8db2ca63cfb90) have internal "raw" functions which are missing the static keyword.

This was causing build failures in our builds in the MicroPython project when updating to the latest LittleFS, because we're building with -Werror,-Wmissing-prototypes,-Wmissing-declarations. See here and here for the build output.

This PR adds the static keyword to these functions to fix these, similar to how all other internal functions in LittleFS are defined.

geky-bot commented 8 months ago
Tests passed ✓, Code: 16820 B (-0.1%), Stack: 1448 B (+0.0%), Structs: 800 B (+0.0%) | | Code | Stack | Structs | | Coverage | |:--|-----:|------:|--------:|:--|---------:| | Default | 16820 B (-0.1%) | 1448 B (+0.0%) | 800 B (+0.0%) | Lines | 2357/2533 lines (+0.0%) | | Readonly | 6130 B (+0.0%) | 448 B (+0.0%) | 800 B (+0.0%) | Branches | 1202/1528 branches (+0.0%) | | Threadsafe | 17688 B (-0.2%) | 1448 B (+0.0%) | 808 B (+0.0%) | | **Benchmarks** | | Multiversion | 16884 B (-0.1%) | 1448 B (+0.0%) | 804 B (+0.0%) | Readed | 29369693876 B (+0.0%) | | Migrate | 18500 B (-0.1%) | 1752 B (+0.0%) | 804 B (+0.0%) | Proged | 1482874766 B (+0.0%) | | Error-asserts | 17480 B (-0.1%) | 1440 B (+0.0%) | 800 B (+0.0%) | Erased | 1568888832 B (+0.0%) |
geky commented 8 months ago

Hi @DvdGiessen, thanks for this! Looks like quite an oversight.

Will bring this in next patch release.

geky commented 8 months ago

I've gone ahead and added -Wmissing-prototypes into our Makefile (https://github.com/littlefs-project/littlefs/pull/884/commits/8f3f32d1f31e9b233a22eafa04257b5c9a073160), it seems like a useful warning. This should now be caught by the CI here and prevented in the future.

geky-bot commented 8 months ago
Tests passed ✓, Code: 16820 B (-0.1%), Stack: 1448 B (+0.0%), Structs: 800 B (+0.0%) | | Code | Stack | Structs | | Coverage | |:--|-----:|------:|--------:|:--|---------:| | Default | 16820 B (-0.1%) | 1448 B (+0.0%) | 800 B (+0.0%) | Lines | 2357/2533 lines (+0.0%) | | Readonly | 6130 B (+0.0%) | 448 B (+0.0%) | 800 B (+0.0%) | Branches | 1202/1528 branches (+0.0%) | | Threadsafe | 17688 B (-0.2%) | 1448 B (+0.0%) | 808 B (+0.0%) | | **Benchmarks** | | Multiversion | 16884 B (-0.1%) | 1448 B (+0.0%) | 804 B (+0.0%) | Readed | 29369693876 B (+0.0%) | | Migrate | 18500 B (-0.1%) | 1752 B (+0.0%) | 804 B (+0.0%) | Proged | 1482874766 B (+0.0%) | | Error-asserts | 17480 B (-0.1%) | 1440 B (+0.0%) | 800 B (+0.0%) | Erased | 1568888832 B (+0.0%) |
geky commented 8 months ago

Should be merged now, thanks again for this!