From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from eggs.gnu.org (eggs.gnu.org [IPv6:2001:470:142:3::10]) by sourceware.org (Postfix) with ESMTPS id 63CD53858C66 for ; Fri, 17 Nov 2023 12:10:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 63CD53858C66 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gnu.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gnu.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 63CD53858C66 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:470:142:3::10 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700223003; cv=none; b=YvoMwyXiN9/1IEZiZPLEjotTFV1UxDy0XAm7er7njk0BcC4D3H6VXbPnKxXpypJ1YYW3hhBG0WdDEbyZQ51Ym2iRipmtnbcq6Fy+odtm0NkuuTIgQeMFUMibUkJic3BbqENgqU2qaGdgxaIP4xkb63mntcysgoOdxNwZPKERt9c= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700223003; c=relaxed/simple; bh=YHfuJ7tDqqIvpwJlOqLjjzEwZ0FDHB7iwLUgFRVsYQs=; h=DKIM-Signature:Date:Message-Id:From:To:Subject:MIME-version; b=GiMf9QwOyGKUXws9PCgNY3PUIosl3zGi/wj6Wh9sgGBMaZgb2lyHoz/uVnnRPFDqHhVVCx59YuTawEVxfjRRDIauPnSO4dDlMQJeEzgR8wBp6cze083pnZJ0RoXkF2c13VIL4qYo0/dwd0nSm7S6VFTZXFFGRX28+XZei90cw+8= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r3xfc-0003cv-Ag; Fri, 17 Nov 2023 07:10:00 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=ebqrjELXGW/RiwEw1r6hNg3E0pC3d+fmVmp0zF32n0c=; b=Bc/ilHzcXaD65tXsBW/y id3WiZ7bBvYy8LILQRo9+PXx/d2CHeHWy6Fgc4b5rD17idn2RXfxW2IeCZa2eGdHQmNBE/zH1wX72 eiR5aiTgHWFI1ZbmeN+t/AAx5EGrowWmS74LTvbC78Z/IvactLXh7IXW8PLz7rNOvdl5mBpjLteYP Dts54RYTZoTwLrZJEDhmziMGrfAjJxlzIC14ftoN/WjxJwmhiuNYMiVCVT0apCGhp2apP84vciiZC zbw//xlKfGVCrEFN6zW9IeAhU0Vsnj7R7VrZQ1huIxiqAkVMGvVn6DJ0low8iCEeOT4INPJx0fbu/ qlOGQVvtH4Be6w==; Date: Fri, 17 Nov 2023 14:09:53 +0200 Message-Id: <83v8a0oa26.fsf@gnu.org> From: Eli Zaretskii To: Alexandra =?utf-8?B?SMOhamtvdsOh?= Cc: gdb-patches@sourceware.org In-Reply-To: <20231117111840.2040709-4-ahajkova@redhat.com> (message from Alexandra =?utf-8?B?SMOhamtvdsOh?= on Fri, 17 Nov 2023 12:18:37 +0100) Subject: Re: [PATCH 3/6] Add new vDefaultInferiorFd packet References: <20231117111840.2040709-1-ahajkova@redhat.com> <20231117111840.2040709-4-ahajkova@redhat.com> MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=0.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: > From: Alexandra Hájková > Cc: ahajkova@redhat.com > Date: Fri, 17 Nov 2023 12:18:37 +0100 > > Add a new DefaultInferiorFd feature and the corresponding packet. > This feature allows GDB to send, to GDBserver, the file descriptor > numbers of the terminal to which GDB is connected. The inferior is > then started connected to the same terminal as GDB. This allows the > inferior run by local GDBserver to read from GDB's STDIN and write > its output to GDB's STOUT/ERR the same way as native target. > --- > gdb/doc/gdb.texinfo | 32 +++++++++++ > gdb/remote.c | 28 ++++++++++ > gdbserver/server.cc | 132 +++++++++++++++++++++++++++++++++++++++++++- > gdbserver/server.h | 12 ++++ > 4 files changed, 201 insertions(+), 3 deletions(-) Thanks. I have some comments: > +When GDBserver is run locally using stdio for example > +@code{target remote | gdbserver - inferior}. > +Inferior's STDIN is closed which means the inferior can't read any input. This should be a single sentence, and it currently lacks some mandatory punctuation and uses sub-optimal markup. Here's a suggestion to fix that: When GDBserver is run locally using stdio, for example, with @w{@kbd{target remote | gdbserver - @var{inferior}}}, the inferior's @code{stdin} is closed, which means the inferior can't read any input. > +If GDBserver is run locally using stdio and supports the @xref{default inferior > +file descriptors} feature, GDBserver will start inferior connected to the same > +terminal as @value{GDBN} which enables the inferior to read from STDIN and > +write to the STDOUT/ERR in the same fashion native target does. Suggest to reword like this: If GDBserver supports the default inferior file descriptors feature (@pxref{default inferior file descriptors}), it will start the inferior connected to the same terminal as @value{GDBN}, which enables the inferior to read from @code{stdin} and write to @code{stdout} and @code{stderr}, in the same way native targets do. > +@item vDefaultInferior;@var{fd0};@var{fd1};@var{fd2} > +@item vDefaultInferiorFd The second @item should be @itemx. > +@anchor{default inferior file descriptors} > +@cindex @samp{vDefaultInferiorFd} packet Please place the @anchor and the @cindex lines before the @item lines, so that following the index link will show the @item lines, not only the text below them. > +@value{GDBN} would preserve the numbers of STDOUT/STDIN/STDERR file descriptors. ^^^^^^^^^^^^^^^^^^^ @code{stdin}/@code{stdout}/@code{stderr} And I don't think I understand why we need to mention that GDB preserves these file descriptors. is this important forf using GDB and gdbserver in these cases? > +Preserved numbers of the file descriptors are then sent to GDBserver. Same question about this sentence: is it important, and of so, why? > +If GDBserver is run locally and will accept the numbers of the file > +descriptors, it will start the inferior connected to the same STDIN/OUT/ERR > +@value{GDBN} is connected to. This allows the inferior run under the local GDBserver > +to read from the STDIN and write to STOUT/ERR. This makes a remote target (which is > +run locally) to behave similarly to the native target. I'm confused here: does the above passage describe an alternative way of starting the inferior? If so, how is that method useful, and how can the user control the behavior? Also, please leave two spaces, not one, between sentences, per our conventions. > +This feature does not support setting inferior tty. What does this mean in practice? Which commands and/or packets are not supported, and what does it mean for GDB user in practice? Reviewed-By: Eli Zaretskii