From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 44551 invoked by alias); 7 Apr 2017 17:51:30 -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 44526 invoked by uid 89); 7 Apr 2017 17:51:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_NONE autolearn=no version=3.3.2 spammy= X-HELO: mail-wm0-f42.google.com Received: from mail-wm0-f42.google.com (HELO mail-wm0-f42.google.com) (74.125.82.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 07 Apr 2017 17:51:28 +0000 Received: by mail-wm0-f42.google.com with SMTP id w64so4182690wma.0 for ; Fri, 07 Apr 2017 10:51:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=S0LrA6zGfYUMC3iN72THY8qYwX5INwSQ9eD9/Qdh9y0=; b=gkWYEZ+CywnGRk/KJ3KRq0IqXZjGgcsQGKszlMBPP8+hI/iDf8vzV/aKrI4jZyJFrN K/LRpc0mXLz7qRTS2Dowe6AnKfA6qVkdNdaVTdH3QReK/DDDkwKNQ6InAvkrapB1yG3Q qedUYs3gIKiyXWm/DXfkvifJ3G6PkGEPF4gpEjelVvT35JlerWRcIgfBQMAGYTpXswBz fnaBvpsSKpsUGEZt62am9RmWAEl6BBlPbGG7HrcaXquxN2aDap8U70Lg450n7kil2M0b o8Ph6FrOLFJcAaMAvdKhfz8fbyt76gioW0jBZju+crCnIWDQN/6d3C5snpENBaMEgAWH C/vg== X-Gm-Message-State: AN3rC/7tu3306bGeG0/iqJXCQo5Aqa7kKW2B1oXtdEx/vQz39iRP3ee7 aX821gYgxDcnjNwtWNua8g== X-Received: by 10.28.34.6 with SMTP id i6mr457940wmi.41.1491587488025; Fri, 07 Apr 2017 10:51:28 -0700 (PDT) Received: from [192.168.0.101] ([37.189.166.198]) by smtp.gmail.com with ESMTPSA id y15sm410749wme.27.2017.04.07.10.51.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 07 Apr 2017 10:51:26 -0700 (PDT) Subject: Re: [PATCH v5 1/5] Move parts of inferior job control to common/ To: Sergio Durigan Junior References: <1482464361-4068-1-git-send-email-sergiodj@redhat.com> <20170330014915.4894-1-sergiodj@redhat.com> <20170330014915.4894-2-sergiodj@redhat.com> <68a31f8b-d315-0ae7-6d2b-6811bce05766@redhat.com> <87a881tksh.fsf@redhat.com> <87bmshrvm6.fsf@redhat.com> Cc: GDB Patches From: Pedro Alves Message-ID: Date: Fri, 07 Apr 2017 17:51: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: <87bmshrvm6.fsf@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-04/txt/msg00187.txt.bz2 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 " 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. Thanks, Pedro Alves