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

Pedro Alves
2017-02-15T16:02:10Z

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