Refactoring std os setenv

[darnuria]

Hello!

After talking with @huonw on Irc and trying different thing, I purpose to add a error handling over this wrapper of libc::setenv for catching errors.

I will need some help on the two commit marked as fixup!. I am still a newbie to rust. :)

[huonw]

I think this would be better as fail!("setenv failed: {}", e).

[alexcrichton]

The only documented error case on unix for setenv is ENOMEM. We currently abort on OOM everywhere else in Rust, so it seems a little odd to start worrying about it for this one function (and getting annoying errors about checking the result of setenv).

Are there error cases for windows that we're worried about not catching? If it's just OOM then I'd tend to think that these functions should continue to return ()

[darnuria]

@alexcrichton: Funny on my Linux I have two errors documented.

man setenv on Ubuntu 14.04:

RETURN VALUE
       The setenv() function returns zero on success, or  -1  on  error,  with
       errno set to indicate the cause of the error.
       The  unsetenv()  function returns zero on success, or -1 on error, with
       errno set to indicate the cause of the error.
ERRORS
       EINVAL name is NULL, points to a string of length 0, or contains an '='
              character.
       ENOMEM Insufficient memory to add a new variable to the environment.
CONFORMING TO
       4.3BSD, POSIX.1-2001.
BUGS
       POSIX.1-2001  specifies  that  if  name contains an '=' character, then
       setenv() should fail with the error EINVAL; however, versions of  glibc
       before 2.3.4 allowed an '=' sign in name.

And rustc is compiled with:

GNU C Library (Ubuntu EGLIBC 2.19-0ubuntu6) stable release version 2.19, by Roland McGrath et al.
Copyright (C) 2014 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
PARTICULAR PURPOSE.
Compiled by GNU CC version 4.8.2.
Compiled on a Linux 3.13.9 system on 2014-04-12.
Available extensions:
    crypt add-on version 2.1 by Michael Glad and others
    GNU Libidn by Simon Josefsson
    Native POSIX Threads Library by Ulrich Drepper et al
    BIND-8.2.3-T5B
libc ABIs: UNIQUE IFUNC
For bug reporting instructions, please see:
<https://bugs.launchpad.net/ubuntu/+source/eglibc/+bugs>.

[alexcrichton]

Ah yes, I also had EINVAL as an error listed, but we fail pretty much everywhere else in std::os if a string contains NUL, and I believe that this should remain consistent with the other methods.

[darnuria]

So the best solution is to:

• Drop this pull-request?
• Rewrite some std::os with OsError/IoError in mind?

If somebody wants to write something like a shell in Rust it would be nice to have the possibility to check errors like theses instead of having a internal error.

[alexcrichton]

I would have to look at other functions as well, but failing on NUL is pretty common throughout the codebase, and it may be intentional rather than accidental. For now though, I'm going to close this pending further investigation.

[darnuria]

@alexcrichton https://gist.github.com/darnuria/9312f285ee8b265abf92
Originally I worked on this, because of this case.

[alexcrichton]

That is an interesting case!