Re: Add login_tty

[Rich Felker <dalias@libc.org>]

One more issue:

On Sat, Nov 01, 2014 at 11:27:29PM +0100, Felix Janda wrote:
>  int forkpty(int *m, char *name, const struct termios *tio, const struct winsize *ws)
>  {
> @@ -13,12 +12,7 @@ int forkpty(int *m, char *name, const struct termios *tio, const struct winsize
>  	pid = fork();
>  	if (!pid) {
>  		close(*m);
> -		setsid();
> -		ioctl(s, TIOCSCTTY, (char *)0);
> -		dup2(s, 0);
> -		dup2(s, 1);
> -		dup2(s, 2);
> -		if (s>2) close(s);
> +		login_tty(s);

Here there's no checking for failure of login_tty. Note that checking
for failure would not be useful, because there's no valid response,
but:

> diff --git a/src/misc/login_tty.c b/src/misc/login_tty.c
> new file mode 100644
> index 0000000..f0be0a0
> --- /dev/null
> +++ b/src/misc/login_tty.c
> @@ -0,0 +1,14 @@
> +#include <utmp.h>
> +#include <sys/ioctl.h>
> +#include <unistd.h>
> +
> +int login_tty(int fd)
> +{
> +	setsid();
> +	if (ioctl(fd, TIOCSCTTY, (char *)0)) return -1;
> +	dup2(fd, 0);
> +	dup2(fd, 1);
> +	dup2(fd, 2);
> +	if (fd>2) close(fd);
> +	return 0;
> +}

Unlike the code moved out of forkpty, this code errors out early if
the ioctl fails, thereby skipping the dup2's and close. In the case of
forkpty, this leaves the child process in a very messed-up state with
regard to its file descriptors; it will clobber the parent's
stdin/out/err without knowing anything is wrong.

In the case of forkpty, the error just needs to be ignored, I think,
like it is now. I'm not sure what login_tty should do, though.
Reporting the error is good, but leaving the process and its file
descriptors in an unpredictable state is not good. Is there any
documentation for how they're left on error?

Rich