Re: [PATCH 1/2] unshare,nsenter: spawn shell by default

[Karel Zak <kzak@redhat.com>]

On Thu, Feb 07, 2013 at 07:09:01PM -0500, Zbigniew Jędrzejewski-Szmek wrote:
> +#define DEFAULT_SHELL "/bin/sh"
> +
> +void __attribute__((__noreturn__)) exec_shell(void) {
> +	const char *shell = getenv("SHELL");
> +	if (!shell)
> +		shell = DEFAULT_SHELL;
> +	execl(shell, basename(shell), "-i", NULL);
> +	err(EXIT_FAILURE, _("exec %s failed"), shell);
> +}

 Do we really need "-i", for example su(1) uses

     arg0[0] = '-';
     strcpy (arg0 + 1, shell_basename);

     execv (shell, (char **) args);

 see login-utils/su-common.c

 man bash:

   A login shell is one whose first character of argument zero is a -,
   or one started with the --login option.

 Not sure, but I guess that "-basename" as argv[0] is more portable
 solution (it's originally from coreutils, it has to be portable :-).

> +	if (optind < argc)
> +		execvp(argv[optind], argv + optind);
> +	else
> +		exec_shell();
>  
>  	err(EXIT_FAILURE, _("failed to execute %s"), argv[optind]);

 It would be more readable:

   if (optind < argc) {
        execvp(argv[optind], argv + optind);
        err(EXIT_FAILURE, _("failed to execute %s"), argv[optind]);
   }
   exec_shell();

   
   Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com