[libc-commits] [libc] [libc][stdlib] Add unsetenv (PR #202422)

[Pavel Labath via libc-commits]

https://github.com/labath commented:

Most of the patch is straight-forward, but I'm not sure what to think about the dependency on the global `environ`. I get why it's needed, but this still seems like it violates the "internal code does not depend on entry points rule".

One way to avoid this would be to use some sort of a dependency injection setup, where the environment manager would be initialized with a pointer to the global `environ` (if it is present). That doesn't exactly solve this problem, as there still has to be /something/ which depends on it, but that something could be the startup code (which could maybe be exempted from the cmake warning-spewing logic).

I don't know if that's acceptable/desirable for the project though, which is why I'd feel better if @michaelrj-google (or someone else) took a look at this as well.

Then, there's also the interesting edge case of an environment that contains the same variable twice. The easiest way to construct that is to pass it via `execve`. If I'm reading this implementation this will just peel off one definition of the variable, which will make the other one valid. 

This isn't exactly a standard scenario, so maybe that's acceptable, but I want to note that this is not what's happening with glibc right now:
```
$ cat a.c
#define _GNU_SOURCE
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>

int main(int argc, char **argv) {
  if (argc == 1) {
    char *e[] = {"FOO=bar", "FOO=baz", NULL};
    execle(argv[0], argv[0], "XXX", NULL, e);
    return 1;
  }
  for (char **e = environ; *e; ++e)
    printf("environ: %s\n", *e);
  printf("getenv(\"FOO\")=%s\n", getenv("FOO"));
  unsetenv("FOO");
  puts("======");
  for (char **e = environ; *e; ++e)
    printf("environ: %s\n", *e);
  printf("getenv(\"FOO\")=%s\n", getenv("FOO"));
  return 0;
}
$ gcc -static a.c -o a.out
$ env - strace -e trace=execve -v --  ./a.out 
execve("./a.out", ["./a.out"], [])      = 0
execve("./a.out", ["./a.out", "XXX"], ["FOO=bar", "FOO=baz"]) = 0
environ: FOO=bar
environ: FOO=baz
getenv("FOO")=bar
======
getenv("FOO")=(null)
```

I haven't tried this with musl, but judging by the implementation, I think it will have the same behavior.

https://github.com/llvm/llvm-project/pull/202422