fs: Filesystem module (WIP)

[miri64]

This PR creates a common interface for all filesystems to use.

STILL WIP

[Kijewski]

Strong "no" to the use of errno.

[Kijewski]

Superfluous. The variable is auto initialized with zeroes.

[miri64]

Are you sure?

[Kijewski]

Yes.

[OlegHahm]

What's wrong with errno?

[Kijewski]

I see no advantage in having this as a static inline instead of a regular function.

[miri64]

I thought that all of them will be just wrappers of two function calls, this might save memory.

[Kijewski]

Actually I don't think that this will safe memory, on the contrary. The static inline function is quite big, as it employs to function calls.

[miri64]

Yes, but it goes into the ROM, rather than the RAM, and on most devices ROM >> RAM.

[miri64]

(On the contrary: a non-inline function would build up RAM space through its parameters)

[Kijewski]

I think the static inline function will neither safe RAM nor ROM. :)

[kaspar030]

errno is not thread safe / reentrant.

[OlegHahm]

Does it have to here?

[Kijewski]

There is virtually no advantage in setting errno since it is not reentrant. All reads to the variable are spurious.
The functions should just use a wide enough int type (e.g. ssize_t) to return negated error codes.

[Kijewski]

Where did you find this list? Fuse?

[miri64]

Yes ;-)

[Kijewski]

At many lines the semicola are missing.

[Kijewski]

I like the idea, and think that we will need FS support (and SD card support for MSB-A2) and a proper VFS.
But I think we should not target to implement all these functions. We don't want to become a less well-tested Linux.

[OlegHahm]

There is SD card support for the MSB-A2.

[miri64]

@Kijewski regarding errno: better this way?

[miri64]

Squashed and rebased to master

[Kijewski]

@authmillenon, yes, I like using the return code. 👍

[SGSSGene]

It's not necessary to use strncmp. strcmp (without n) will do the work.

[Kijewski]

Confirmed.
Also, please use strcmp with != 0, because it's not quite a boolean value.

[miri64]

Well the idea is to only compare up to the length of the mount point's path. As far as I understand it strcmp() would yield < 0 if I compare for example fs_table[i].mount_point == "/mnt" with path == "/mnt/where/ever/files/go". But about != 0 you are right

[miri64]

(But it should be == 0 to be equivalent to !strncmp ;-))

[SGSSGene]

I miss understood the function. In that case of course strncmp() :-).

[Kijewski]

Right now you are looking for the last match. This could be easified by reverse iterating the list and exiting eagerly.
But don't you have to find the longest prefix?
And maybe the mount point could memoize it's strlen?

[miri64]

Yes I search for the longest prefix. You are right, I have to save the length, too :/

[Kijewski]

Maybe an all-on-one solution?

size_t is_prefix_len(const char *restrict prefix, const char *restrict str)
{
    size_t len = 0;
    while (1) {
        if (!*prefix) {
            return len;
        }
        else if (*prefix != *str) {
            return 0;
        }
        else {
            ++prefix;
            ++str;
            ++len;
        }
    }
}

[miri64]

see 3cce976 ;-)

[Kijewski]

I like 👍

[miri64]

@kaspar030 FatFS is a full library for FAT for embedded systems. We don't need to do anything, exept providing an interface ;-)

[miri64]

@gebart I guess pkg is the always the best option for external libs, since maintenance in case of updates is easier this way (the avoidance of license hassle is just an extra ;-)).

OT: embunit is currently the only non-package module which was included from an external source. Mainly because it IS license-compatible, since it is pretty integral to our testing infrastructure, since we applied a lot of changes by now and to avoid redownloads on Travis.

[jnohlgard]

@authmillenon FatFS as a pkg sounds like a good solution.

btw, do any of the RIOT boards have an implementation for SDHC so far?

[miri64]

The msba2 has at least a microSD slot, but I don't now if it is SDHC or just SD and if a driver implementation for this exists. Maybe @OlegHahm can shed some light on this.

[miri64]

@gebart if you like you can adopt this PR, btw. I don't know if I come to continue this in the near future. The interface is inspired mainly by fuse's btw (exept that I did not use filenames but fds as identifiers apparently I did use filenames) ;-)

[jnohlgard]

@authmillenon I meant SD host controller interface, sorry about the unclear acronym.

[jnohlgard]

I do not have any time to work on this at the moment either, maybe I will adopt this later when things clear up, but for now this one may be up for grabs.

[miri64]

👍

[waehlisch]

@x3ro started also to think/work about a file system for RIOT. Maybe he can join the discussion.

[x3ro]

Thanks for the ping @waehlisch :) My plan is to be working on bringing SDHC and filesystem support to RIOT over the next couple of months as part of my masters thesis. @authmillenon Would it be an option for me to adopt this PR?

The msba2 has at least a microSD slot, but I don't now if it is SDHC or just SD and if a driver implementation for this exists. @authmillenon

As far as I know there is no such driver. I played with the msba2 at the last Hack&Ack to see if I can talk to the SD card but have so far been unsuccessful.

[miri64]

@authmillenon Would it be an option for me to adopt this PR?

@x3ro, of course. GitHub has no feature however to just let me give it to you. You have to check out my corresponding branch and open your own PR with it. Then we can close this one.

[OlegHahm]

@gebart:

Contiki has CFS, but that is BSD licensed, so we can't use that code.

Why should this be a problem? AFAIK it shouldn't be a problem to include BSD/MIT/... code into a (L)GPL project - but of course not via versa.

@authmillenon:

OT: embunit is currently the only non-package module which was included from an external source.

ccn-lite is also a non-package stemming from an external source - and currently it seems to me that this was a bad idea (although it looked wise back when the decision was originally made). Hence, I would also vote for "packaging" wherever possible.

@gebart @authmillenon @x3ro:

As far as I know there is no such driver. I played with the msba2 at the last Hack&Ack to see if I can talk to the SD card but have so far been unsuccessful.

I used to work with micro SDHC cards on the MSB-A2 and RIOT (though it was still named µkleos back then) about three years ago, which worked more or less fine (see also #127). However, I haven't tested for a looong time.

[jnohlgard]

@OlegHahm

Why should this be a problem? AFAIK it shouldn't be a problem to include BSD/MIT/... code into a (L)GPL project - but of course not via versa.

I am sorry, I did not think this through before I wrote it. You are right, AFAIK we are allowed to use BSD sources within an LGPL project such as this. I also found this document regarding different open source licenses and their interoperability:

http://wiki.osdev.org/Licensing

@authmillenon , @OlegHahm What would be a reasonable way to integrate Contiki's CFS into RIOT?
CFS does not have a separate repo, but is closely integrated into the Contiki repo.

Can we add it as a pkg with some script to fetch a git revision and strip everything except the CFS pieces?

[OlegHahm]

I think that could be one way to do this, but probably not the most elegant way. Maybe we should reach out to the Contiki people and see if we can maintain CFS as an external library together on the long run.

[miri64]

Some thought on file descriptors I had I'd like to share: Maybe any value of kernel_pid_t with > MAXTHREADS is the best solution, since in POSIX even pthreads have fds and this way we can just have them be the actual PID, while anything else is uniquely identified. This way we have a unique numbering scheme and have the POSIX wrapper for pthreads and file operations as easy as possible. Sockets would also grab from the same pool then (and we have the option to run every socket in it's own light-weight thread (if we'll have something like that) in the future, maybe).

[jnohlgard]

@authmillenon watch out for the FDs of stdout, stdin, stderr though. They are defined as 0, 1, and 2 in most implementations.

[miri64]

Damn you're right. It's even specified this way: http://pubs.opengroup.org/onlinepubs/9699919799/functions/stdin.html

[x3ro]

Sorry for my idleness so far 🐌 If nobody objects I would open a new pull request from my fork, adding a summary of the discussion so far to that PR. This thread could then be closed and the discussion would continue in the new PR. Does that make sense?

[jnohlgard]

@x3ro That makes sense.
Just add the text #1265 somewhere in the description of your PR and a back-reference will be added inside this thread to your new PR, to make it easier to follow.

[OlegHahm]

👍

[x3ro]

Can anyone explain what this does/should do? 😕

[jnohlgard]

I think they meant to do return 0; or return 1; or something

[miri64]

I guess this was meant to be this, too.

[OlegHahm]

Should we close this as a memo now?

[miri64]

Yes