From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7783 invoked by alias); 7 Feb 2017 22:39:46 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 7760 invoked by uid 89); 7 Feb 2017 22:39:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=ttys, types.h, typesh, UD:types.h X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 07 Feb 2017 22:39:35 +0000 Received: from smtp.corp.redhat.com (int-mx16.intmail.prod.int.phx2.redhat.com [10.5.11.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AED2F804E0; Tue, 7 Feb 2017 22:39:35 +0000 (UTC) Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4C95BACBAF; Tue, 7 Feb 2017 22:39:35 +0000 (UTC) From: Sergio Durigan Junior To: Luis Machado Cc: GDB Patches , Pedro Alves , Eli Zaretskii Subject: Re: [PATCH v2 2/6] Share parts of gdb/terminal.h with gdbserver References: <1482464361-4068-1-git-send-email-sergiodj@redhat.com> <20170118153605.4610-1-sergiodj@redhat.com> <20170118153605.4610-3-sergiodj@redhat.com> <90c917a2-e6e8-2384-7be2-412ed48a5500@codesourcery.com> X-URL: http://blog.sergiodj.net Date: Tue, 07 Feb 2017 22:39:00 -0000 Message-ID: <87tw854od6.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-02/txt/msg00170.txt.bz2 Hey Luis, Thanks for the review. Comments below. On Wednesday, February 01 2017, Luis Machado wrote: > On 01/18/2017 09:36 AM, Sergio Durigan Junior wrote: >> As part of the bigger work of sharing fork_inferior with gdbserver, >> some parts of gdb/terminal.h also needed to be moved to a common >> place. These parts are: >> >> - The code responsible for determining some terminal-based define's >> based on available features; >> >> - job control; >> >> - terminal-related functions needed by fork_inferior; >> >> gdb/ChangeLog: >> 2017-01-17 Sergio Durigan Junior >> >> * Makefile.in (HFILES_NO_SRCDIR): Add "common/common-terminal.h". >> * common/common-terminal.h: New file, with parts of "terminal.h". >> * terminal.h: Move terminal-related defines to >> "common/common-terminal.h". Include "common/common-terminal.h". >> (new_tty_prefork): Move to "common/common-terminal.h". >> (new_tty): Likewise. >> (new_tty_postfork): Likewise. >> (job_control): Likewise. >> (create_tty_session): Likewise. >> (gdb_setpgid): Likewise. >> * utils.c: Include "terminal.h". >> >> gdb/gdbserver/ChangeLog: >> 2016-12-22 Sergio Durigan Junior >> >> * terminal.c: New file. >> --- >> gdb/Makefile.in | 1 + >> gdb/common/common-terminal.h | 119 +++++++++++++++++++++++++++++++++++++++++++ >> gdb/gdbserver/terminal.c | 88 ++++++++++++++++++++++++++++++++ >> gdb/terminal.h | 73 +------------------------- >> gdb/utils.c | 1 + >> 5 files changed, 210 insertions(+), 72 deletions(-) >> create mode 100644 gdb/common/common-terminal.h >> create mode 100644 gdb/gdbserver/terminal.c >> >> diff --git a/gdb/Makefile.in b/gdb/Makefile.in >> index 3f19818..c05d456 100644 >> --- a/gdb/Makefile.in >> +++ b/gdb/Makefile.in >> @@ -1469,6 +1469,7 @@ HFILES_NO_SRCDIR = \ >> common/common-regcache.h \ >> common/common-types.h \ >> common/common-utils.h \ >> + common/common-terminal.h \ >> common/errors.h \ >> common/environ.h \ >> common/fileio.h \ >> diff --git a/gdb/common/common-terminal.h b/gdb/common/common-terminal.h >> new file mode 100644 >> index 0000000..f7ab96a >> --- /dev/null >> +++ b/gdb/common/common-terminal.h >> @@ -0,0 +1,119 @@ >> +/* Common terminal interface definitions for GDB and gdbserver. >> + Copyright (C) 1986-2017 Free Software Foundation, Inc. >> + >> + This file is part of GDB. >> + >> + This program is free software; you can redistribute it and/or modify >> + it under the terms of the GNU General Public License as published by >> + the Free Software Foundation; either version 3 of the License, or >> + (at your option) any later version. >> + >> + This program is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + GNU General Public License for more details. >> + >> + You should have received a copy of the GNU General Public License >> + along with this program. If not, see . */ >> + >> +#ifndef COMMON_TERMINAL_H >> +#define COMMON_TERMINAL_H >> + >> +/* If we're using autoconf, it will define HAVE_TERMIOS_H, >> + HAVE_TERMIO_H and HAVE_SGTTY_H for us. One day we can rewrite >> + ser-unix.c and inflow.c to inspect those names instead of >> + HAVE_TERMIOS, HAVE_TERMIO and the implicit HAVE_SGTTY (when neither >> + HAVE_TERMIOS or HAVE_TERMIO is set). Until then, make sure that >> + nothing has already defined the one of the names, and do the right >> + thing. */ >> + >> +#if !defined (HAVE_TERMIOS) && !defined(HAVE_TERMIO) && !defined(HAVE_SGTTY) >> +#if defined(HAVE_TERMIOS_H) >> +#define HAVE_TERMIOS >> +#else /* ! defined (HAVE_TERMIOS_H) */ >> +#if defined(HAVE_TERMIO_H) >> +#define HAVE_TERMIO >> +#else /* ! defined (HAVE_TERMIO_H) */ >> +#if defined(HAVE_SGTTY_H) >> +#define HAVE_SGTTY >> +#endif /* ! defined (HAVE_SGTTY_H) */ >> +#endif /* ! defined (HAVE_TERMIO_H) */ >> +#endif /* ! defined (HAVE_TERMIOS_H) */ >> +#endif /* !defined (HAVE_TERMIOS) && !defined (HAVE_TERMIO) && >> + !defined (HAVE_SGTTY) */ >> + >> +#if defined(HAVE_TERMIOS) >> +#include >> +#endif >> + >> +#if !defined(_WIN32) && !defined (HAVE_TERMIOS) >> + >> +/* Define a common set of macros -- BSD based -- and redefine whatever >> + the system offers to make it look like that. FIXME: serial.h and >> + ser-*.c deal with this in a much cleaner fashion; as soon as stuff >> + is converted to use them, can get rid of this crap. */ >> + >> +#ifdef HAVE_TERMIO >> + >> +#include >> + >> +#undef TIOCGETP >> +#define TIOCGETP TCGETA >> +#undef TIOCSETN >> +#define TIOCSETN TCSETA >> +#undef TIOCSETP >> +#define TIOCSETP TCSETAF >> +#define TERMINAL struct termio >> + >> +#else /* sgtty */ >> + >> +#include >> +#include >> +#include >> +#define TERMINAL struct sgttyb >> + >> +#endif /* sgtty */ >> +#endif >> + >> +#include >> + >> +/* Do we have job control? Can be assumed to always be the same >> + within a given run of GDB. Use in gdb/inflow.c and >> + common/common-inflow.c. */ >> +extern int job_control; >> + >> +extern void new_tty (void); >> + >> +/* NEW_TTY_PREFORK is called before forking a new child process, >> + so we can record the state of ttys in the child to be formed. >> + TTYNAME is null if we are to share the terminal with gdb; > > This is still referencing only gdb, but the code is now shared with > gdbserver as well. > >> + or points to a string containing the name of the desired tty. >> + >> + NEW_TTY is called in new child processes under Unix, which will >> + become debugger target processes. This actually switches to >> + the terminal specified in the NEW_TTY_PREFORK call. */ >> +extern void new_tty_prefork (const char *ttyname); >> + >> +/* NEW_TTY_POSTFORK is called after forking a new child process, and >> + adding it to the inferior table, to store the TTYNAME being used by >> + the child, or null if it sharing the terminal with gdb. */ > > Here as well. > >> +extern void new_tty_postfork (void); >> + >> +/* Create a new session if the inferior will run in a different tty. >> + A session is UNIX's way of grouping processes that share a controlling >> + terminal, so a new one is needed if the inferior terminal will be >> + different from GDB's. > > And here. These are actually right (for now, at least), since they are just used by GDB and their gdbserver counterpart are just placeholders. I will expand the comment to mention that. >> + >> + Returns the session id of the new session, 0 if no session was created >> + or -1 if an error occurred. */ >> +extern pid_t create_tty_session (void); >> + >> +/* Set the process group of the caller to its own pid, or do nothing >> + if we lack job control. */ >> +extern int gdb_setpgid (void); >> + >> +/* Determine whether we have job control, and set variable JOB_CONTROL >> + accordingly. */ >> +extern void have_job_control (void); >> + >> +#endif /* ! COMMON_TERMINAL_H */ >> diff --git a/gdb/gdbserver/terminal.c b/gdb/gdbserver/terminal.c >> new file mode 100644 >> index 0000000..42ac651 >> --- /dev/null >> +++ b/gdb/gdbserver/terminal.c >> @@ -0,0 +1,88 @@ >> +/* Terminal interface definitions for the GDB remote server. >> + Copyright (C) 2016-2017 Free Software Foundation, Inc. >> + >> + This file is part of GDB. >> + >> + This program is free software; you can redistribute it and/or modify >> + it under the terms of the GNU General Public License as published by >> + the Free Software Foundation; either version 3 of the License, or >> + (at your option) any later version. >> + >> + This program is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + GNU General Public License for more details. >> + >> + You should have received a copy of the GNU General Public License >> + along with this program. If not, see . */ >> + >> +#include "server.h" >> +#include "common-terminal.h" >> + >> +/* See common/common-terminal.h. */ >> + >> +void >> +new_tty (void) >> +{ >> + /* Placeholder needed by fork_inferior. For now, this function is >> + not needed nor useful to have on gdbserver. When/If we properly >> + handle terminal modes, we can revisit and implement the needed >> + support. */ >> +} >> + >> +/* See common/common-terminal.h. */ >> + >> +void >> +new_tty_prefork (const char *ttyname) >> +{ >> + /* Placeholder needed by fork_inferior. For now, this function is >> + not needed nor useful to have on gdbserver. When/If we properly >> + handle terminal modes, we can revisit and implement the needed >> + support. */ >> +} >> + >> +/* See common/common-terminal.h. */ >> + >> +void >> +new_tty_postfork (void) >> +{ >> + /* Placeholder needed by fork_inferior. For now, this function is >> + not needed nor useful to have on gdbserver. When/If we properly >> + handle terminal modes, we can revisit and implement the needed >> + support. */ >> +} >> + >> +/* See common/common-terminal.h. */ >> + >> +pid_t >> +create_tty_session (void) >> +{ >> + /* Placeholder needed by fork_inferior. For now, this function is >> + not needed nor useful to have on gdbserver. When/If we properly >> + handle terminal modes, we can revisit and implement the needed >> + support. */ >> + return (pid_t) 1; >> +} >> + >> +/* See common/common-inferior.h. */ >> + >> +void >> +set_inferior_io_terminal (const char *terminal_name) >> +{ >> + /* Placeholder needed by fork_inferior. For now, this function is >> + not needed nor useful to have on gdbserver. When/If we properly >> + handle terminal modes, we can revisit and implement the needed >> + support. */ >> +} >> + >> +/* See common/common-inferior.h. */ >> + >> +const char * >> +get_inferior_io_terminal (void) >> +{ >> + /* Placeholder needed by fork_inferior. For now, this function is >> + not needed nor useful to have on gdbserver. When/If we properly >> + handle terminal modes, we can revisit and implement the needed >> + support. */ >> + return NULL; >> +} >> diff --git a/gdb/terminal.h b/gdb/terminal.h >> index d8691b2..8139319 100644 >> --- a/gdb/terminal.h >> +++ b/gdb/terminal.h >> @@ -19,83 +19,12 @@ >> #if !defined (TERMINAL_H) >> #define TERMINAL_H 1 >> >> - >> -/* If we're using autoconf, it will define HAVE_TERMIOS_H, >> - HAVE_TERMIO_H and HAVE_SGTTY_H for us. One day we can rewrite >> - ser-unix.c and inflow.c to inspect those names instead of >> - HAVE_TERMIOS, HAVE_TERMIO and the implicit HAVE_SGTTY (when neither >> - HAVE_TERMIOS or HAVE_TERMIO is set). Until then, make sure that >> - nothing has already defined the one of the names, and do the right >> - thing. */ >> - >> -#if !defined (HAVE_TERMIOS) && !defined(HAVE_TERMIO) && !defined(HAVE_SGTTY) >> -#if defined(HAVE_TERMIOS_H) >> -#define HAVE_TERMIOS >> -#else /* ! defined (HAVE_TERMIOS_H) */ >> -#if defined(HAVE_TERMIO_H) >> -#define HAVE_TERMIO >> -#else /* ! defined (HAVE_TERMIO_H) */ >> -#if defined(HAVE_SGTTY_H) >> -#define HAVE_SGTTY >> -#endif /* ! defined (HAVE_SGTTY_H) */ >> -#endif /* ! defined (HAVE_TERMIO_H) */ >> -#endif /* ! defined (HAVE_TERMIOS_H) */ >> -#endif /* !defined (HAVE_TERMIOS) && !defined (HAVE_TERMIO) && >> - !defined (HAVE_SGTTY) */ >> - >> -#if defined(HAVE_TERMIOS) >> -#include >> -#endif >> - >> -#if !defined(_WIN32) && !defined (HAVE_TERMIOS) >> - >> -/* Define a common set of macros -- BSD based -- and redefine whatever >> - the system offers to make it look like that. FIXME: serial.h and >> - ser-*.c deal with this in a much cleaner fashion; as soon as stuff >> - is converted to use them, can get rid of this crap. */ >> - >> -#ifdef HAVE_TERMIO >> - >> -#include >> - >> -#undef TIOCGETP >> -#define TIOCGETP TCGETA >> -#undef TIOCSETN >> -#define TIOCSETN TCSETA >> -#undef TIOCSETP >> -#define TIOCSETP TCSETAF >> -#define TERMINAL struct termio >> - >> -#else /* sgtty */ >> - >> -#include >> -#include >> -#include >> -#define TERMINAL struct sgttyb >> - >> -#endif /* sgtty */ >> -#endif >> +#include "common-terminal.h" >> >> struct inferior; >> >> -extern void new_tty_prefork (const char *); >> - >> -extern void new_tty (void); >> - >> -extern void new_tty_postfork (void); >> - >> extern void copy_terminal_info (struct inferior *to, struct inferior *from); >> >> -/* Do we have job control? Can be assumed to always be the same within >> - a given run of GDB. In inflow.c. */ >> -extern int job_control; >> - >> -extern pid_t create_tty_session (void); >> - >> -/* Set the process group of the caller to its own pid, or do nothing if >> - we lack job control. */ >> -extern int gdb_setpgid (void); >> - >> /* Set up a serial structure describing standard input. In inflow.c. */ >> extern void initialize_stdin_serial (void); >> >> diff --git a/gdb/utils.c b/gdb/utils.c >> index f142ffe..4993145 100644 >> --- a/gdb/utils.c >> +++ b/gdb/utils.c >> @@ -53,6 +53,7 @@ >> #include "top.h" >> #include "main.h" >> #include "solist.h" >> +#include "terminal.h" >> >> #include "inferior.h" /* for signed_pointer_to_address */ >> >> > 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/