From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24151 invoked by alias); 2 Nov 2017 19:27:26 -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 24139 invoked by uid 89); 2 Nov 2017 19:27:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= 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; Thu, 02 Nov 2017 19:27:23 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 827C313A55 for ; Thu, 2 Nov 2017 19:27:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 827C313A55 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id A37ED60BE0; Thu, 2 Nov 2017 19:27:19 +0000 (UTC) Subject: Re: [PATCH 1/3] Assume termios is available, remove support for termio and sgtty To: Sergio Durigan Junior References: <1509635522-16945-1-git-send-email-palves@redhat.com> <1509635522-16945-2-git-send-email-palves@redhat.com> <87r2tgcxzv.fsf@redhat.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: Date: Thu, 02 Nov 2017 19:27:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <87r2tgcxzv.fsf@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-11/txt/msg00037.txt.bz2 On 11/02/2017 06:54 PM, Sergio Durigan Junior wrote: > On Thursday, November 02 2017, Pedro Alves wrote: > >> This commit garbage collects the termio and sgtty support. >> >> GDB's terminal handling code still has support for the old termio and >> sgtty interfaces in addition to termios. However, I think it's pretty >> safe to assume that for a long, long time, Unix-like systems provide >> termios. GNU/Linux, Solaris, Cygwin, AIX, DJGPP, macOS and the BSDs >> all have had termios.h for many years. Looking around the web, I >> found discussions about FreeBSD folks trying to get rid of old sgtty.h >> a decade ago: >> >> https://lists.freebsd.org/pipermail/freebsd-hackers/2007-March/019983.html >> >> So I think support for termio and sgtty in GDB is just dead code that >> is never compiled anywhere and is just getting in the way. For >> example, serial_noflush_set_tty_state and the raw<->cooked concerns >> mentioned in inflow.c only exist because of sgtty (see >> hardwire_noflush_set_tty_state). >> >> Regtested on GNU/Linux. >> >> Confirmed that I can still build Solaris, DJGPP and AIX GDB and that >> that GDB still includes the termios.h-guarded code. Confirmed > > "that that" Eh, that was actually on purpose. It's "I confirmed that $something", with "$something == that GDB (the one I built) still includes". The first 'that' is the subordinating that, and the second 'that' is a demonstrative pronoun. It's not unlike: "I think that this thing is blue, and I think that that other thing is white". > >> mingw-w64 GDB still builds and skips the termios.h-guarded code. > > Thanks for doing that. > > You may remember that I also stumbled upon this while doing the > startup-with-shell, but I didn't have the necessary background knowledge > to do a deep cleanup. It's always nice to see old code being removed. Well, it was only after banging my head against the terminal handling for a couple weeks that I realized that this is all dead code. :-P >> --- a/gdb/Makefile.in >> +++ b/gdb/Makefile.in >> @@ -729,12 +729,9 @@ XMLFILES = \ >> $(srcdir)/features/traceframe-info.dtd \ >> $(srcdir)/features/xinclude.dtd >> >> -# This is ser-unix.o for any system which supports a v7/BSD/SYSV/POSIX >> -# interface to the serial port. Hopefully if get ported to OS/2, VMS, >> -# etc., then there will be (as part of the C library or perhaps as >> -# part of libiberty) a POSIX interface. But at least for now the >> -# host-dependent makefile fragment might need to use something else >> -# besides ser-unix.o >> +# This is ser-unix.o for any system which supports a POSIX interface >> +# to the serial port. The host-dependent makefile fragment might need >> +# to use something else besides ser-unix.o. >> SER_HARDWIRE = @SER_HARDWIRE@ >> >> # The `remote' debugging target is supported for most architectures, > > You also need to remove common/gdb_termios.h from HFILES_NO_SRCDIR here. Good point. Consider it done. >> -#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 >> @@ -56,7 +58,6 @@ gdb_setpgid () >> #endif >> #endif /* HAVE_SETPGRP */ >> #endif /* HAVE_SETPGID */ >> -#endif /* defined (HAVE_TERMIOS) || defined (TIOCGPGRP) */ > > Did you decide to remove the check for HAVE_TERMIOS (instead of > replacing it with an "#ifdef HAVE_TERMIOS_H") because we're already > checking for HAVE_SETPGID? Yes. >> --- a/gdb/gdbserver/remote-utils.c >> +++ b/gdb/gdbserver/remote-utils.c >> @@ -17,7 +17,9 @@ >> along with this program. If not, see . */ >> >> #include "server.h" >> -#include "gdb_termios.h" >> +#if HAVE_TERMIOS_H >> +#include >> +#endif >> #include "target.h" >> #include "gdbthread.h" >> #include "tdesc.h" >> @@ -325,7 +327,7 @@ remote_open (const char *name) >> if (remote_desc < 0) >> perror_with_name ("Could not open remote device"); >> >> -#ifdef HAVE_TERMIOS >> +#if HAVE_TERMIOS_H > > Why s/#ifdef/#if/? I prefer #ifdef BTW. Yeah, I used #if here because it's what the file is already using for the other headers: #if HAVE_SYS_IOCTL_H #include #endif #if HAVE_SYS_FILE_H #include #endif etc. > As far as I could, I reviewed all the changes, and they seem OK to me > modulo the few nits I pointed. Thanks for the review! -- Pedro Alves