Re: [PATCH 2/6] gdb/ser-pipe.c: Duplicate the file descriptors

[Tom Tromey <tom@tromey.com>]

>>>>> "Alexandra" == Alexandra Hájková <ahajkova@redhat.com> writes:

Alexandra> Duplicate the numbers of STDOUT/STDIN/STDERR file descriptors
Alexandra> GDB is connected to. Preserved numbers of the file descriptors
Alexandra> could be then sent to the GDBserver. If GDBserver is run locally
Alexandra> and will accept he numbers of the file descriptors, it can start
Alexandra> the inferior connected to the same STDIN/OUT/ERR, GDB is connected to.

Thanks for the patch.

Alexandra> +  /* Preserve STDIN/STDOUT/STDERR so they won't be closed on
Alexandra> +     exec later, after we fork.  */
Alexandra> +  int saved_stdin = dup (STDIN_FILENO);
Alexandra> +  int saved_stdout = dup (STDOUT_FILENO);
Alexandra> +  int saved_stderr = dup (STDERR_FILENO);

I wonder what should happen if any of these fail.

Alexandra> @@ -128,6 +144,10 @@ pipe_open (struct serial *scb, const char *name)
Alexandra>  	  close (err_pdes[1]);
Alexandra>  	}
 
Alexandra> +      mark_fd_no_cloexec (saved_stdout);
Alexandra> +      mark_fd_no_cloexec (saved_stdin);
Alexandra> +      mark_fd_no_cloexec (saved_stderr);
Alexandra> +

This happens in the child process, but due to vfork, it actually affects
the parent process as well.  vfork is weird.

However I think this introduces a funny bug, in that these particular
descriptors are marked as no-cloexec by number.  So, while they are
closed again in the parent:

Alexandra> +  close (saved_stdout);
Alexandra> +  close (saved_stdin);
Alexandra> +  close (saved_stderr);

... their numbers are preserved for not-closing, and so it's possible
they could be reused and inherited by accident by some future
subprocess.

Probably these closes in the parent should be paired with calls to
unmark_fd_no_cloexec.

I'd suggest hoisting the calls to mark_fd_no_cloexec near the dup()s,
too, just to make it more clear.

Tom