Skip to content

Conversation

daniel-j
Copy link

@daniel-j daniel-j commented Nov 20, 2024

  • Wrap _ftello_r as it collides when linking.
  • Use PATH_MAX from limits.h instead.
  • Add support for reading file modified timestamp using stat (fatfs only).

LittleFS could also support file timestamps through attributes although this would have to be implemented as an extra feature.

I'm not using pico-vfs from C directly but I made a Nim wrapper for it that I'm working on in my Pico SDK wrapper
https://github.com/daniel-j/picostdlib/blob/master/src/picostdlib/pico/filesystem.nim
https://github.com/daniel-j/picostdlib/tree/master/examples/filesystem

_ftello_r linking error:

/usr/lib/gcc/arm-none-eabi/14.1.0/../../../../arm-none-eabi/bin/ld: /usr/lib/gcc/arm-none-eabi/14.1.0/../../../../arm-none-eabi/lib/thumb/v6-m/nofp/libg.a(libc_a-ftello.o): in function `_ftello_r':
/build/arm-none-eabi-newlib/src/build-newlib/arm-none-eabi/thumb/v6-m/nofp/newlib/../../../../../../newlib-4.4.0.20231231/newlib/libc/stdio/ftello.c:88: multiple definition of `_ftello_r'; CMakeFiles/test_pico.dir/picostdlib/src/picostdlib/vendor/pico-vfs/src/filesystem/vfs.c.obj (symbol from plugin):(.text+0x0): first defined here

Nice work on pico-vfs!

@oyama
Copy link
Owner

oyama commented Apr 9, 2025

@daniel-j Thank you for the pull request. Sorry for the very late reply.

Overall it looks good, I agree with using FS_PATH_MAX in FatFs. However, I think it is better to use the size of PATH_MAX in limits.h of arm-none-eabi to reserve the buffer, since the VFS layer can use much larger paths depending on the file system.

@daniel-j
Copy link
Author

Hi, I have updated the PR to use limits.h instead. Thanks!

Copy link
Owner

@oyama oyama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1
Thanks!

@daniel-j
Copy link
Author

I have updated again with a fix for _ftello_r, have a look and see if you like it. Also fixed an issue with month number in stat mtime.

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.

2 participants