NuxiNL / cloudabi

Definitions for the CloudABI data types and system calls
https://nuxi.nl/cloudabi/
Other
131 stars 5 forks source link

i32 vs u32 #9

Open mcandre opened 5 years ago

mcandre commented 5 years ago

I think file descriptors tend to use signed 32-bit integers, whereas the Rust crate uses unsigned. How important is it to expand the potential open files that wide, versus the trade-off of losing some type compatibility between libc vs. CloudABI?

This subtle difference makes it a bit harder to write polyglot programs that can target libc and CloudABI, depending on the toolchain applied.

m-ou-se commented 5 years ago

@EdSchouten Is there a specific reason you defined fd as unsigned here? https://github.com/NuxiNL/cloudabi/blob/a3ae008b1c0a1f33a3764fff69607afb16d45135/cloudabi.txt#L408

Edit: Oh, looks like I wrote that line. I'm pretty sure I simply copied the type from your original C headers though.

EdSchouten commented 5 years ago

I think we picked uint32 for the reason that CloudABI system calls don't return errors in band. The file descriptors returned by syscalls are never reused to return errors. They are opaque types, just like HANDLE on Windows.

Though I think that uint32 is technically correct in our case, I do agree that this is causing some churn when mixing pure POSIX and CloudABI code. Even inside of cloudlibc this is causing some nastiness (e.g., the casts in program_exec(), program_spawn(), recvmsg(), sendmsg() to convert between int * <-> cloudabi_fd_t *).

On the other hand, using uint32 does have the advantage that it's a bit safer on the kernel/runtime side. Observe this code in cloudabi-run's emulator:

static cloudabi_errno_t fd_table_get_entry(struct fd_table *ft,
                                           cloudabi_fd_t fd,
                                           cloudabi_rights_t rights_base,
                                           cloudabi_rights_t rights_inheriting,
                                           struct fd_entry **ret)
    REQUIRES_SHARED(ft->lock) {
  // Test for file descriptor existence.
  if (fd >= ft->size)
    return CLOUDABI_EBADF;
  struct fd_entry *fe = &ft->entries[fd];
  ...

This code will need to be extended if cloudabi_fd_t is altered. Feel free to send a pull request, but please take some time to audit the existing kernel/runtime implementations when changing this. Thanks!