Re: [PATCH v5 1/5] Move parts of inferior job control to common/

Sergio Durigan Junior
2017-04-12T00:25:29Z

On Friday, April 07 2017, Pedro Alves wrote:

> On 03/31/2017 10:20 PM, Sergio Durigan Junior wrote:
>> On Friday, March 31 2017, Pedro Alves wrote:
>
>>> So that .h file contains both definitions of symbols that
>>> are the exported API of job-control.c, and also the whole
>>> termios.h/termio.h #ifdefery mess.
>>>
>>> It doesn't look like the termios.h stuff is needed by
>>> the _clients_ of the functions defined in job-control.c?
>>>
>>> So I think the best is to have two headers instead.
>>>
>>> One with the termios.h/termio.h #ifdef-ery stuff,
>>> called common/gdb_termios.h. That's be similar in spirit
>>> to gdb/gdb_curses.h. That should only then be included
>>> in files that actually need to see those bits defined.
>>> Maybe job-control.c and inflow.c and not much else.
>>>
>>> The "#include <sys/types.h>" include should probably not be
>>> in the resulting gdb_termios.h. I can't see why it's there
>>> in your common-terminal.h.
>>>
>>> And then add a new common/job-control.h header that matches the
>>> new job-control.c file, that _only_ exports job-control.c's
>>> public interface, like, these three declarations:
>>>
>>> extern int job_control;
>>> extern int gdb_setpgid ();
>>> extern void have_job_control ();
>>
>> Thanks, it makes sense to separate like this, indeed.
>>
>>> Also, it looks like gdbserver's terminal.h has all the same
>>> termios goo that you were putting in common-terminal.h.
>>> We should remove/undup all that.
>>
>> Yeah, after a closer look I noticed that the code is just a simplified
>> version of what's already done on the GDB side (now under common/). So
>> I went ahead and deleted the gdbserver file.
>>
>> Thanks for this review, I implemented everything you mentioned on this
>> message but will wait until you review the other parts before I send the
>> v6.
>
> Note that if you extract the
> gdb/terminal.h / gdbserver/terminal.h
> => common/gdb_termios.h
> part (including the corresponding common.m4 bits) into its own preparatory
> patch, and post it separately, we can get that merged and out of the way
> quickly. That part stands on its own right.

OK, I'll do that now.

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible