From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 90394 invoked by alias); 2 Nov 2017 18:34:19 -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 90335 invoked by uid 89); 2 Nov 2017 18:34:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=sk:startup 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 18:34:08 +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 8A68561D0A for ; Thu, 2 Nov 2017 18:34:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 8A68561D0A Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=sergiodj@redhat.com Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5DE0760BE5; Thu, 2 Nov 2017 18:34:07 +0000 (UTC) From: Sergio Durigan Junior To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/3] Don't set terminal flags twice in a row References: <1509635522-16945-1-git-send-email-palves@redhat.com> <1509635522-16945-3-git-send-email-palves@redhat.com> Date: Thu, 02 Nov 2017 18:34:00 -0000 In-Reply-To: <1509635522-16945-3-git-send-email-palves@redhat.com> (Pedro Alves's message of "Thu, 2 Nov 2017 15:12:01 +0000") Message-ID: <87vaiscyxt.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg00035.txt.bz2 On Thursday, November 02 2017, Pedro Alves wrote: > I find this odd 'set flags twice' ancient code and comment annoyingly > distracting. It may well be that the reason for the double-set was > simply a copy/paste mistake, and that we've been doing this for > decades [1] for no good reason. Let's just get rid of it, and if we > find a real reason, add it back with a comment explaining why it's > necessary. > > [1] This double-set was already in gdb 2.4 / 1988, the oldest release > we have sources for, and imported in git. From 'git show 7b4ac7e1ed2c > inflow.c': > > +void > +terminal_inferior () > +{ > + if (terminal_is_ours) /* && inferior_thisrun_terminal == 0) */ > + { > + fcntl (0, F_SETFL, tflags_inferior); > + fcntl (0, F_SETFL, tflags_inferior); > > The "is there a reason" comment was added in 1993, by: > > commit a88797b5eadf31e21804bc820429028bf708fbcd > Author: Fred Fish > AuthorDate: Thu Aug 5 01:33:45 1993 +0000 FWIW, I've stumbled upon this part while doing the startup-with-shell work, and it also intrigued me. Anyway, I agree that we should remove this double-set. Thanks for doing that. > > gdb/ChangeLog: > yyyy-mm-dd Pedro Alves > > * inflow.c (child_terminal_inferior, child_terminal_ours_1): No > longer set flags twice in row. > --- > gdb/inflow.c | 9 --------- > 1 file changed, 9 deletions(-) > > diff --git a/gdb/inflow.c b/gdb/inflow.c > index a96d4fc..d46d693 100644 > --- a/gdb/inflow.c > +++ b/gdb/inflow.c > @@ -244,10 +244,6 @@ child_terminal_inferior (struct target_ops *self) > int result; > > #ifdef F_GETFL > - /* Is there a reason this is being done twice? It happens both > - places we use F_SETFL, so I'm inclined to think perhaps there > - is some reason, however perverse. Perhaps not though... */ > - result = fcntl (0, F_SETFL, tinfo->tflags); > result = fcntl (0, F_SETFL, tinfo->tflags); > OOPSY ("fcntl F_SETFL"); > #endif > @@ -403,11 +399,6 @@ child_terminal_ours_1 (int output_only) > > #ifdef F_GETFL > tinfo->tflags = fcntl (0, F_GETFL, 0); > - > - /* Is there a reason this is being done twice? It happens both > - places we use F_SETFL, so I'm inclined to think perhaps there > - is some reason, however perverse. Perhaps not though... */ > - result = fcntl (0, F_SETFL, our_terminal_info.tflags); > result = fcntl (0, F_SETFL, our_terminal_info.tflags); > #endif > } > -- > 2.5.5 -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/