From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 60114 invoked by alias); 19 Jul 2019 08:50:40 -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 60101 invoked by uid 89); 19 Jul 2019 08:50:40 -0000 Authentication-Results: sourceware.org; auth=none 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,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-wr1-f65.google.com Received: from mail-wr1-f65.google.com (HELO mail-wr1-f65.google.com) (209.85.221.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 19 Jul 2019 08:50:39 +0000 Received: by mail-wr1-f65.google.com with SMTP id p17so31394167wrf.11 for ; Fri, 19 Jul 2019 01:50:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=hG2rbAuohJcp25UUlQmZGDL+y+BxVMkh+F4HDc+ar0s=; b=WvcfhR84nMY7FGtgsx16eb1988qZZgAGh2VlEy5Ou/7hG7ATPHBNUg2vxe/zhBhlY+ 3Unto1daDFlKt10eNE+00yxoi9fdtqsN/KVreOqvJ2F8VQD0ISzZaNOTavjaDgdvSu5e C98mr92KVlA5Uyxxy4ilPJ/BXKrHDRlEBEsx5lnMt5V+qYOzmLnDkfFcXbCkneECV13k BrNvX6Qo5R1qNisX2AzOIIgxB8E1j3Sua7fJ15WOaRzGtE6qGyVAP2XnLZ7O4HcyHmH+ de6fwgzOxQcg8dlNVb+ZXT2g03DGdFEMPpQdcn11gGIiYoApzDogoSrkRhI4fblsr7jH 2DTw== Return-Path: Received: from localhost (host86-171-250-129.range86-171.btcentralplus.com. [86.171.250.129]) by smtp.gmail.com with ESMTPSA id g2sm27207760wmh.0.2019.07.19.01.50.36 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 19 Jul 2019 01:50:36 -0700 (PDT) Date: Fri, 19 Jul 2019 08:50:00 -0000 From: Andrew Burgess To: Pedro Alves Cc: LABARTHE Guillaume , gdb-patches@sourceware.org Subject: Re: [RFC][PATCH] new-ui command under windows using NamedPipe Message-ID: <20190719085035.GX23204@embecosm.com> References: <7c1c5343-3171-d454-8507-507d5b8a24a5@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7c1c5343-3171-d454-8507-507d5b8a24a5@redhat.com> X-Fortune: Lusers learning curve appears to be fractal X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2019-07/txt/msg00448.txt.bz2 * Pedro Alves [2019-07-18 17:24:34 +0100]: > On 7/17/19 7:41 PM, LABARTHE Guillaume wrote: > > Hello, > > > > I am currently working on a front-end for gdb on windows, and trying > > to use the new-ui command passing as tty name the name of a named pipe > > without luck. > > > > Then I decided to dig into it so I cloned gdb's repo and started > > debugging. After some investigation I found out that the problem was > > that the function new_ui_command in top.c opens the same tty three > > times (for stdin, sdout and stderr). With windows named pipes the > > second and third calls to open fail. I then patched the function to > > open the file only once and pass the same stream for stdin stdout and > > stderr and that made it work. > > Wow, awesome! I've been saying for years now that named pipes is > probably the way to go for Windows. I had no idea the fix would be > so simple! > > > > > I don't know the implication of my patch on other operating systems or > > what would be the way to make it specific to windows. > > I tried it on GNU/Linux and things still work. > > I ran all the MI tests with forced new-ui, with: > > $ make check TESTS="gdb.mi/*.exp" RUNTESTFLAGS="FORCE_MI_SEPARATE_UI=1" > > and saw no regressions. Are all of these special flags for testing documented anywhere? Every now and then someone will mention some new flag that I've never seen before and I always think that one day it might be helpful... Thanks, Andrew > > > > > Can you please advise on the best way to make this patch portable. > > You will find in the attachments my patch so far. > It actually looks good as is. I wrote a ChangeLog, synthesized a > commit log, tweaked the comments a little bit, and merged it as below. > > Note: we're currently leaking the terminal file if the UI is closed, > but that happens without your patch as well. > > From afe09f0b6311a4dd1a7e2dc6491550bb228734f8 Mon Sep 17 00:00:00 2001 > From: Guillaume LABARTHE > Date: Thu, 18 Jul 2019 17:20:04 +0100 > Subject: [PATCH] Fix for using named pipes on Windows > > On Windows, passing a named pipe as terminal argument to the new-ui > command does not work. > > The problem is that the new_ui_command function in top.c opens the > same tty three times, for stdin, stdout and stderr. With Windows > named pipes, the second and third calls to open fail. > > Opening the file only once and passing the same stream for stdin, > stdout and stderr makes it work. > > Pedro says: > > I tried it on GNU/Linux and things still work. > I ran all the MI tests with forced new-ui, with: > > $ make check TESTS="gdb.mi/*.exp" RUNTESTFLAGS="FORCE_MI_SEPARATE_UI=1" > > and saw no regressions. > > gdb/ChangeLog: > 2019-07-18 Guillaume LABARTHE > > * top.c (new_ui_command): Open specified terminal just once. > --- > gdb/ChangeLog | 4 ++++ > gdb/top.c | 18 +++++++----------- > 2 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index fa669daa4b3..d6fe9895a71 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,7 @@ > +2019-07-18 Guillaume LABARTHE > + > + * top.c (new_ui_command): Open specified terminal just once. > + > 2019-07-18 Tom Tromey > > * symtab.c (main_name): Constify return type. > diff --git a/gdb/top.c b/gdb/top.c > index 83a3604688b..60f81b3bf85 100644 > --- a/gdb/top.c > +++ b/gdb/top.c > @@ -337,8 +337,6 @@ open_terminal_stream (const char *name) > static void > new_ui_command (const char *args, int from_tty) > { > - gdb_file_up stream[3]; > - int i; > int argc; > const char *interpreter_name; > const char *tty_name; > @@ -357,13 +355,13 @@ new_ui_command (const char *args, int from_tty) > { > scoped_restore save_ui = make_scoped_restore (¤t_ui); > > - /* Open specified terminal, once for each of > - stdin/stdout/stderr. */ > - for (i = 0; i < 3; i++) > - stream[i] = open_terminal_stream (tty_name); > + /* Open specified terminal. Note: we used to open it three times, > + once for each of stdin/stdout/stderr, but that does not work > + with Windows named pipes. */ > + gdb_file_up stream = open_terminal_stream (tty_name); > > std::unique_ptr ui > - (new struct ui (stream[0].get (), stream[1].get (), stream[2].get ())); > + (new struct ui (stream.get (), stream.get (), stream.get ())); > > ui->async = 1; > > @@ -373,10 +371,8 @@ new_ui_command (const char *args, int from_tty) > > interp_pre_command_loop (top_level_interpreter ()); > > - /* Make sure the files are not closed. */ > - stream[0].release (); > - stream[1].release (); > - stream[2].release (); > + /* Make sure the file is not closed. */ > + stream.release (); > > ui.release (); > } > -- > 2.14.5 >