Re: [PATCH] nsenter: new command (light wrapper around setns)

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1: Type: Text/Plain, Size: 1610 bytes --]

On Friday 11 January 2013 17:46:38 Eric W. Biederman wrote:
> --- /dev/null
> +++ b/sys-utils/nsenter.c
>
> +#ifndef CLONE_NEWSNS
> +# define CLONE_NEWNS 0x00020000
> +#endif
> +#ifndef CLONE_NEWUTS
> +# define CLONE_NEWUTS 0x04000000
> +#endif
> +#ifndef CLONE_NEWIPC
> +# define CLONE_NEWIPC 0x08000000
> +#endif
> +#ifndef CLONE_NEWNET
> +# define CLONE_NEWNET 0x40000000
> +#endif
> +#ifndef CLONE_NEWUSER
> +# define CLONE_NEWUSER 0x10000000
> +#endif
> +#ifndef CLONE_NEWPID
> +# define CLONE_NEWPID 0x20000000
> +#endif

this logic really should be in a common header and shared with unshare.c

> +static struct namespace_file{
> +	int nstype;
> +	char *name;

const

> +	/* Careful the order is signifcant in this array.

significant

> +static void open_target_fd(int *fd, const char *type, char *path)

path should be const

> +			namespace_target_pid = strtoul(optarg, &end, 10);
> +			if (!*optarg || (*optarg && *end) || errno != 0) {
> +				err(EXIT_FAILURE,
> +				    _("Pid '%s' is not a valid number"),
> +				    optarg);
> +			}

we really should introduce an xstroul() helper

> +	if (do_fork) {
> +		pid_t child = fork();
> +		if (child < 0)
> +			err(EXIT_FAILURE, _("fork failed"));
> +		if (child != 0) {
> +			int status;
> +			if ((waitpid(child, &status, 0) == child) &&
> +			     WIFEXITED(status)) {
> +				exit(WEXITSTATUS(status));
> +			}
> +			exit(EXIT_FAILURE);

shouldn't this inform the user if the child was killed by a signal ?  maybe:
	if (WIFSIGNALED(status))
		kill(getpid(), WTERMSIG(status));

normalizing the status into EXIT_FAILURE doesn't seem right ...
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]