littlefs-project / littlefs

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

Provide option to use string arguments that are not null-terminated #934

Open BSven opened 8 months ago

BSven commented 8 months ago

I would highly appreciate to have the option to use explicit length specifications for string parameters in all API functions.

Example for open(): I would be great to not only have: int lfs_file_open(lfs_t *lfs, lfs_file_t *file, const char *path, int flags); ... where path is assumed to be null-terminated, but also: int lfs_file_open(lfs_t *lfs, lfs_file_t *file, const char *path, lfs_size_t path_length, int flags); ... where the length of path is specified explicitely.

geky commented 8 months ago

Hi @BSven, this should be doable. We would just need to change the path parsing code (lfs_dir_find) to not rely on null-terminated strings. Unfortunately the null-terminator is useful for writing parsers. Curiously this often gets overlooked when discussing their tradeoffs.

One question is what would these functions be named? lfs_file_opennotnullterminated? lfs_file_opennnt?

BSven commented 8 months ago

Hi @geky, thanks for your immediate reply and your constructive outlook!

I would go with lfs_file_open_nnt so that the main function (i.e., open) stands out clearly.

geky commented 8 months ago

thanks for your immediate reply and your constructive outlook!

Well, to be clear this will be low-priority relative to other implementation-level issues, of which there is a bit of a long list.

I would go with lfs_file_open_nnt so that the main function (i.e., open) stands out clearly.

I get where you're coming from, but right now littlefs's API only uses underscores to indicate the relevant struct namespace. Is this a weird holdout from old C? maybe. though I do think it helps delineate what the verb is. Either way it's a bit out of scope to change the naming convention of the whole API and ultimately consistency is more important.

I think the wildest this gets with all current proposals is lfs_file_lopennntatcfg.


Other names could be lfs_file_openz or lfs_file_openn.

I'm sort of leaning towards lfs_file_openn to somewhat vaguely relate to strncp and friends. Though it is curious how little prior-art there is.

geky commented 8 months ago

Lua does have pushlstring/pushstring [ref], but an l suffix risks confusion with lstat if we ever have symbolic links (way low priority). Though having openl, lstat, and lseek be three wildly different modifiers would make for a pretty funny API.

BSven commented 8 months ago

The first thing I tried when thinking about the name was to somehow use the "n"-pattern, as in the string functions (e.g., strncpy, strncmp) but I did not manage to come up with an idea that felt well. Thinking about that again, lfs_file_openn feels pretty good now. From all the listed candidates (that satisfy the conventions), this whould be my winner so far.

Viatorus commented 7 months ago

Another alternative which comes to my mind would be to use a compile option for enabling/disabeling the use of char + strlen or a struct holding char + len.

Something like this:

#ifndef LFS_NO_STRLN
int lfs_file_open(lfs_t *lfs, lfs_file_t *file, const char *path, int flags);

#else
struct lfs_string {
  const char* str;
  size_t len;
}

int lfs_file_open(lfs_t *lfs, lfs_file_t *file, const lfs_string *path, int flags);
#endif

A caller than has to provide the struct, which shouldn't be a problem at all, because the lifetime of the struct only needs to be as long as the function call.

struct lfs_string path;
path.str = "my message"
path.len = 10;
lfs_file_open(lfs, file, &path, flags);

This way, someone who needs the security of not relying on strlen cannot use it accidentally.

geky commented 7 months ago

Using a compile-time option to choose this string type is a nice idea.

But it becomes a problem if you ever end up with two libraries both using littlefs behind the scenes, one with null-terminated strings, one without. You could duplicate the library at that point, but that's not a reasonable solution for a library intending to be low code-cost.