-
Notifications
You must be signed in to change notification settings - Fork 819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add some easier util overrides: LFS_MALLOC/FREE/CRC #909
Conversation
Now you can override littlefs's malloc with some simple defines: -DLFS_MALLOC=my_malloc -DLFS_FREE=my_free This is probably what most users expected when wanting to override malloc/free in littlefs, but it hasn't been available, since instead littlefs provides a file-level override of builtin utils. The thinking was that there's just too many builtins that could be overriden, lfs_max/min/alignup/npw2/etc/etc/etc, so allowing users to just override the util file provides the best flexibility without a ton of ifdefs. But it's become clear this is awkward for users that just want to replace malloc. Maybe the original goal was too optimistic, maybe there's a better way to structure this file, or maybe the best API is just a bunch of ifdefs, I have no idea! This will hopefully continue to evolve.
Now you can override littlefs's CRC implementation with some simple defines: -DLFS_CRC=lfs_crc The motivation for this is the same for LFS_MALLOC/LFS_FREE. I think these are the main "system-level" utils that users want to override. Don't override with this something that's not CRC32! Your filesystem will no longer be compatible with other tools! This is only intended for provided hardware acceleration!
Tests passed ✓, Code: 16820 B (+0.0%), Stack: 1448 B (+0.0%), Structs: 800 B (+0.0%)
|
It seems this feature is one step short to being useful. Simply defining the macros results in an implicitly declared function, which the compiler then assumes is returning an
I need to provide declarations of the functions, in case of FreeRTOS: extern void * pvPortMalloc(size_t xSize);
extern void vPortFree(void * pv); So basically this means I need to revert to copying |
Hmm, this is an interesting problem, thanks for pointing it out. I think I only tested with symbols defined in system headers, so I didn't notice definitions would be a problem. Unfortunately I'm not sure there's an easy solution. You can include arbitrary files in GCC/Clang with -include, though I think this is generally discouraged because of the impact on build times. If you control the build system you may be able to limit this to only littlefs's sources. Worst case copying |
I was thinking it would be nice to allow user to include their own header through a define, a bit like But I understand it's impossible to cover everyone's use cases and you have to draw a line somewhere. |
Yeah, I guess the question is what would make that include file different from Maybe there's value in allowing an include file that is just prefixed to lfs_util.h. Such an include file could #define LFS_MALLOC, etc, directly, instead of relying on |
Yes, it would allow to keep the default functionality as opposed to the current "all or nothing" approach. |
With this you can override littlefs's malloc/crc implementations with some simple defines:
I think these are the main "system-level" utils that users want to override, but am open to feedback.
Note on LFS_CRC:
Overriding LFS_CRC with something that's not CRC32 is discouraged. Your filesystem will no longer be compatible with other tools. This is only intended for providing hardware acceleration, etc.
These are probably what most users expected when wanting to override malloc/free/crc in littlefs, but they haven't been available, since instead littlefs provides a file-level override of builtin utils behind LFS_CONFIG.
The thinking was that there's just too many builtins that could be overriden, lfs_max/min/alignup/npw2/etc/etc/etc, so allowing users to just override the util file provides the best flexibility without a ton of ifdefs.
But it's become clear this is awkward for users that just want to replace malloc/crc.
Maybe the original goal was too optimistic, maybe there's a better way to structure this file, or maybe the best API is just a bunch of ifdefs, I have no idea. This will hopefully continue to evolve.