From patchwork Thu Jul 18 16:24:34 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 33732 Received: (qmail 120106 invoked by alias); 18 Jul 2019 16:24:42 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 120077 invoked by uid 89); 18 Jul 2019 16:24: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 autolearn=ham version=3.3.1 spammy=HX-Spam-Relays-External:209.85.128.66, H*RU:209.85.128.66 X-HELO: mail-wm1-f66.google.com Received: from mail-wm1-f66.google.com (HELO mail-wm1-f66.google.com) (209.85.128.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 18 Jul 2019 16:24:38 +0000 Received: by mail-wm1-f66.google.com with SMTP id s15so4951006wmj.3 for ; Thu, 18 Jul 2019 09:24:38 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id c7sm22382838wro.70.2019.07.18.09.24.35 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Thu, 18 Jul 2019 09:24:35 -0700 (PDT) Subject: Re: [RFC][PATCH] new-ui command under windows using NamedPipe To: LABARTHE Guillaume , gdb-patches@sourceware.org References: From: Pedro Alves Message-ID: <7c1c5343-3171-d454-8507-507d5b8a24a5@redhat.com> Date: Thu, 18 Jul 2019 17:24:34 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: 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. > > 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 (); }