Add support for the <sys/mman.h> functions by wrapping around our own implementations. There are no kern_*() variants of these system calls, but we also don't need them in this case. It is sufficient to just call into the sys_*() functions.
Details
- Reviewers
brooks emaste - Commits
- rS285652: Implement CloudABI memory management system calls.
These system calls already pass the CloudABI test suite.
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Looks good to me.
sys/compat/cloudabi/cloudabi_mem.c | ||
---|---|---|
108 ↗ | (On Diff #6809) | Would it make sense to kassert fd == -1 on this case and just an unconditional assignment? |
sys/compat/cloudabi/cloudabi_mem.c | ||
---|---|---|
108 ↗ | (On Diff #6809) | These arguments come directly from userspace, so we shouldn't attach KASSERTs to them, right? That way the user can trigger a panic. This is actually where Linux and BSD differ: on Linux, if MAP_ANON is set, the fd argument is ignored. BSD requires the fd to be -1 in that case. For CloudABI I thought it would make sense to relax this requirement. |
sys/compat/cloudabi/cloudabi_mem.c | ||
---|---|---|
108 ↗ | (On Diff #6809) | Right, I realized kassert was wrong later that night. I don't like the inherent sloppyness of allowing any random integer to be passed for "don't-use-this". It seems like people will end up failing late due to calling mem_map(fd_i_want_to_use, ...) with MAP_ANON due to copy and paste errors. Better IMO to fail entirely. |
sys/compat/cloudabi/cloudabi_mem.c | ||
---|---|---|
108 ↗ | (On Diff #6809) | Yes. Now that I think of it, that sounds like a good idea. So one thing I didn't mention: cloudabi_fd_t is actually an unsigned type in CloudABI, for the reason that the system call interface almost never overloads values with error codes. The userspace system call wrappers always return the file descriptor by reference and return a cloudabi_errno_t. There is no need to express negative file descriptor numbers: https://github.com/NuxiNL/cloudlibc/blob/master/src/common/syscalldefs_mi.h#L330 This means that we'd better not test against -1 in this piece of code, as we should just treat the file descriptor as some kind of opaque value. I've just extended cloudlibc's mmap() function to enforce that fd == -1 in this case: https://github.com/NuxiNL/cloudlibc/blob/master/src/libc/sys/mman/mmap.c#L23 In other words, we still enforce it, but it doesn't need to be done in the kernel. Does this look all right to you? |
sys/compat/cloudabi/cloudabi_mem.c | ||
---|---|---|
108 ↗ | (On Diff #6809) | That suggests there should be a (subject to spelling) #define CLOUDABI_MEM_MAP_ANON_FD (uint32_t)-1 Otherwise people are going to have to cast when mapping anon memory (I think). |
sys/compat/cloudabi/cloudabi_mem.c | ||
---|---|---|
108 ↗ | (On Diff #6809) | Done! |