This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 3/6] Share parts of gdb/inflow.c with gdbserver
Thanks for the review. Comments below.
On Wednesday, February 15 2017, Pedro Alves wrote:
> 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.
Good point, I hadn't thought about that. I renamed the file to
common/job-control.c, because even though it also contains terminal
related stuff, the main purpose of the functions there is to help with
job control.
>> 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.
Done.
>> +
>> +#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.
Done.
>> +
>> + 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,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/