Re: [RFC] Provide a file system API


Andy Ross
 

Some random kibitzing. I'm new, so apologies if some of this has been
discussed before:

int fs_open(ZFILE *zfp, const char *file_name, const char *mode);
Why a stdio-style mode string that has to be parsed and not a
fcntl-style flag word which seems like a better fit for the OS? And
to be serious: do we even care about mode? It seems like runtime
checking that you aren't writing to a read-only descriptor is maybe
senseless complexity. And what's the use case for the OS checking
file permissions on Zephyr?

int fs_error(ZFILE *zfp);
What's the use case for an ferror() equivalent? This is a hack in the
C library to work around edge cases of errno not being convenient to
check across multiple I/O calls when the underlying thing might be
something other than a disk. Can't you just arrange things here such
that all functions return a synchronous error code (a quick scan says
you already have) and skip this?

int fs_eof(ZFILE *zfp);
Likewise this doesn't seem useful in an API where everything is known
to be a disk file. Just check fs_tell(p)>=fs_size(p). Though I guess
if that's the implementation internally then it's sane enough just to
leave this as a convenience function.

long fs_size(ZFILE *zfp);
Should return size_t (or ssize_t if an error is possible, I guess).

int fs_init(void);
Do we really need this? There are other initialization hooks you can
use like mount and open. Why must there be a global one?

int fs_format(void);
That doesn't look useful as-is. Even FAT32 has a bunch of options you
are going to want to set in a real-world application (filesystem
label, cluster size, etc...). Realistically maybe this wants to be
provided by a lower-level filesystem driver and not an abstraction
layer? (Which is the way unix does it -- mkfs sits on top of the
block device and not stdio). Or at least pass a string or something
for configuration data that the filesystem can interpret.

int fs_mount(void);
Similarly this wants more context if it's going to be more than a toy.
Right now it's just a synonym for fs_init(), right? There's no
practical difference detectable to a user between the states, they
just need to blindly call two functions with no arguments if they want
"filesystem". And in the future they'll always need to do so,
blindly, because there's nothing else the API gives them. Real apps
are going to want to do things like detect a card insertion and
remount, have more than one filesystem, etc... And this won't evolve
in that direction without an API change.

If you're going to have this, at least pass some configuration
(e.g. three strings for device/directory/options, etc...) to make it
futureproof.

int fs_readdir(ZDIR *zdp, struct dir_entry *entry);
Where's the definition for stuct dir_entry?

Andy

Join devel@lists.zephyrproject.org to automatically receive all group messages.