Re: [PATCH v3 3/6] Share parts of gdb/inflow.c with gdbserver

[Pedro Alves <palves at redhat dot com>]

On 02/08/2017 03:22 AM, Sergio Durigan Junior wrote:

> gdb/ChangeLog:
> yyyy-mm-dd  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	* Makefile.in (SFILES): Add "common/common-inflow.c".
> 	(COMMON_OBS): Add "common/common-inflow.o".
> 	* common/common-inflow.c: New file, with contents from
> 	"gdb/inflow.c".
> 	* inflow.c (gdb_setpgid): Move to "common/common-inflow.c".
> 	(_initialize_inflow): Move setting of "job_control" to
> 	"handle_job_control".
> 	* utils.c (job_control): Delete.
> 
> gdb/gdbserver/ChangeLog:
> yyyy-mm-dd  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	* Makefile.in: Add rule for "common-inflow.o".
> 	(SFILE): Add "common/common-inflow.c".
> 	(OBS): Add "common-inflow.o".


We should take the opportunity to rename the file to something that
makes a more sense wrt to its contents.  "inflow.c" in gdb is
horribly named, I think simply due to code evolution.  It used to
contain, many many years ago the (from the top of the file):

/* Low level interface to ptrace, for GDB when running under Unix.

but everything related to low level ptrace stuff moved away, and
all it was left with was the terminal/job control stuff.

Maybe call it common/job-control.c or maybe something even more
to the point or something like that.

> diff --git a/gdb/common/common-inflow.c b/gdb/common/common-inflow.c
> new file mode 100644
> index 0000000..9871b5e
> --- /dev/null
> +++ b/gdb/common/common-inflow.c
> @@ -0,0 +1,91 @@
> +/* Low level interface to ptrace, for GDB and gdbserver when running under Unix.

Please update this.  The file really has nothing to do with ptrace.

> +
> +#include "common-defs.h"
> +#include "common-terminal.h"
> +
> +/* Nonzero if we have job control.  */
> +int job_control;
> +
> +/* This is here because this is where we figure out whether we (probably)
> +   have job control.  Just using job_control only does part of it because
> +   setpgid or setpgrp might not exist on a system without job control.
> +   It might be considered misplaced (on the other hand, process groups and
> +   job control are closely related to ttys).


This "misplaced" comment could use some updating.  This is no longer
next to the tty stuff.

> +
> +   For a more clean implementation, in libiberty, put a setpgid which merely
> +   calls setpgrp and a setpgrp which does nothing (any system with job control
> +   will have one or the other).  */
> +
> +int
> +gdb_setpgid (void)
> +{
> +  int retval = 0;
> +
> +  if (job_control)
> +    {
> +#if defined (HAVE_TERMIOS) || defined (TIOCGPGRP)
> +#ifdef HAVE_SETPGID
> +      /* The call setpgid (0, 0) is supposed to work and mean the same
> +         thing as this, but on Ultrix 4.2A it fails with EPERM (and
> +         setpgid (getpid (), getpid ()) succeeds).  */
> +      retval = setpgid (getpid (), getpid ());
> +#else
> +#ifdef HAVE_SETPGRP

Thanks,
Pedro Alves