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

[Sergio Durigan Junior <sergiodj@redhat.com>]

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
http://sergiodj.net/