env: don't panic when name is empty

[thomasqueirozb]

No description provided.

[thomasqueirozb]

While looking at the file I also saw this:

coreutils/src/uu/env/src/env.rs

Lines 267 to 268 in 1b39a10

	// FIXME: this should just use execvp() (no fork()) on Unix-like systems
	match Command::new(&*prog).args(args).status() {

I don't think the FIXME is actually necessary. On unix like systems, Command::spawn does indeed fork here

But it looks like it only forks if self.posix_spawn (which calls libc::posix_spawnp) doesn't pass the necessary requirements to be used.

The forked child process calls self.do_exec (which calls libc::execvp after doing some stuff and performing some checks).

If both methods fail, Command::spawn returns an io error.

All of this is basically to say that the fork only happens in some circumstances and that calling the standard library is a better way to do this than calling execvp directly, since it does a lot of error checking and tries to recover when something fails. The env utility shouldn't have to worry about a fork and we should just keep things as they are.

[miDeb]

Thanks for the PR! Nice that you added such a detailed comment and a test :)

[miDeb]

All of this is basically to say that the fork only happens in some circumstances

Indeed, on my system it calls clone (which is called by posix_spawnp) according to strace . However, I think the main point of this comment is that it would not be necessary to spawn a child process and that we could keep using the current process. I think it's probably not worth changing, as you pointed out using the standard library is much simpler.

[thomasqueirozb]

Thanks for the PR! Nice that you added such a detailed comment and a test :)

I spent a lot of time researching what was the correct way to do this. Might as well document it and possibly save someone else's time in the future :)

All of this is basically to say that the fork only happens in some circumstances

Indeed, on my system it calls clone (which is called by posix_spawnp) according to strace . However, I think the main point of this comment is that it would not be necessary to spawn a child process and that we could keep using the current process. I think it's probably not worth changing, as you pointed out using the standard library is much simpler.

Will remove the FIXME and change the comment to specify why fork/clone is called.

[miDeb]

Thanks again!