From patchwork Wed Jul 30 18:26:14 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 2237 Received: (qmail 6674 invoked by alias); 30 Jul 2014 18:26:20 -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 6660 invoked by uid 89); 30 Jul 2014 18:26:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f201.google.com Received: from mail-pd0-f201.google.com (HELO mail-pd0-f201.google.com) (209.85.192.201) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 30 Jul 2014 18:26:17 +0000 Received: by mail-pd0-f201.google.com with SMTP id g10so294454pdj.2 for ; Wed, 30 Jul 2014 11:26:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-type; bh=p3Px4ahzEQnzRM6u6WJ27bSs/XkSIS6FfbhtStIwCpE=; b=e09jngTjHXXprp1F8bQ3hDUQNZlUlSAMCyVM6QxKmBLk6TkSMI2JB+rLYsowSHTU0K Rpj+GYbUvJ3uiEgJjcVsVrv8ug/5DqOHcOYFSS9PaeC4ijh73V8aJfKJeEF2/5VgS0z0 qSWXvyaVkMiTrea7xjvXiyF1OYpw3vTz+Wxzh3F1AKHmhbYNNSgYu/20Ae4SFt/lU5Xu 4c4MS9UlaKqQurJM2v7HJA26NQCAJ15oHmyy4E1AlTvM8SYuHFETfS00psqo5FdPYPtu BRHKHqwiAJgC9lE+Ztr0xFi3flmOL6XL+Xx2XWaARgaE++xDodQlnnMhyIe6GYASaR6c s9Rw== X-Gm-Message-State: ALoCoQltAFrVpf+DxTdDW7EYZTgAVCEFK0tb/D+BuxbI8tZVE4kguw0e7TDOkz/H/n1zK4j8CdcFohzr4Ai1CKJ941UGAbJQIrJ3prBS6/tNsdVYdjWaMQ2X39FxcKXGuGVw334x8wV4 X-Received: by 10.66.218.162 with SMTP id ph2mr2462218pac.3.1406744775800; Wed, 30 Jul 2014 11:26:15 -0700 (PDT) Received: from corp2gmr1-1.hot.corp.google.com (corp2gmr1-1.hot.corp.google.com [172.24.189.92]) by gmr-mx.google.com with ESMTPS id v44si193678yhv.0.2014.07.30.11.26.15 for (version=TLSv1.1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 30 Jul 2014 11:26:15 -0700 (PDT) Received: from ruffy.mtv.corp.google.com (ruffy.mtv.corp.google.com [172.17.128.44]) by corp2gmr1-1.hot.corp.google.com (Postfix) with ESMTP id 66B8C31C4F5 for ; Wed, 30 Jul 2014 11:26:15 -0700 (PDT) From: Doug Evans To: gdb-patches@sourceware.org Subject: [PATCH] Add comments to terminal_ours/inferior functions Date: Wed, 30 Jul 2014 11:26:14 -0700 Message-ID: MIME-Version: 1.0 X-IsSubscribed: yes Hi. inflow.c has this: /* Put some of our terminal settings into effect, enough to get proper results from our output, but do not change into or out of RAW mode so that no input is discarded. After doing this, either terminal_ours or terminal_inferior should be called to get back to a normal state of affairs. */ void child_terminal_ours_for_output (struct target_ops *self) { child_terminal_ours_1 (1); } /* Put our terminal settings into effect. First record the inferior's terminal settings so they can be restored properly later. */ void child_terminal_ours (struct target_ops *self) { child_terminal_ours_1 (0); } which seems ok (ie. to the reader the distinction between "terminal-ours" and "terminal-ours-for-output" is properly handled) ... until you read child_terminal_ours_1 where you find out the output_only distinction is ignored. Eh? /* output_only is not used, and should not be used unless we introduce separate terminal_is_ours and terminal_is_ours_for_output flags. */ static void child_terminal_ours_1 (int output_only) { ... } This is handled/worked-around in the linux implementation. [One could say inf-child.c doesn't handle target-async and leaves it to the target to subclass - e.g. inf-child.c has this: t->to_can_async_p = return_zero;] /* target_terminal_ours implementation. */ static void linux_nat_terminal_ours (struct target_ops *self) { if (!target_is_async_p ()) { /* Async mode is disabled. */ child_terminal_ours (self); return; } /* GDB should never give the terminal to the inferior if the inferior is running in the background (run&, continue&, etc.), but claiming it sure should. */ child_terminal_ours (self); if (async_terminal_is_ours) return; clear_sigint_trap (); add_file_handler (input_fd, stdin_event_handler, 0); async_terminal_is_ours = 1; } I have a patch-in-progress to add async support to child_terminal_*, but it's going to take some work before I think it's ready (still not sure whether to do it at all, but it's interesting to play with). In the interim I'd like to make the current code easier to read/understand for the next time I pass through here. 2014-07-30 Doug Evans * inflow.c (child_terminal_inferior): Add comment. (child_terminal_ours_for_output): Add comment. (child_terminal_ours): Add comment. * linux-nat.c (linux_nat_terminal_inferior): Add comment. (linux_nat_terminal_ours): Add comment. diff --git a/gdb/inflow.c b/gdb/inflow.c index 5f81de2..8d07757 100644 --- a/gdb/inflow.c +++ b/gdb/inflow.c @@ -267,7 +267,11 @@ child_terminal_init (struct target_ops *self) } /* Put the inferior's terminal settings into effect. - This is preparation for starting or resuming the inferior. */ + This is preparation for starting or resuming the inferior. + + N.B. Targets that want to use this with async support must build that + support on top of this (e.g., the caller still needs to add stdin to the + event loop). E.g., see linux_nat_terminal_inferior. */ void child_terminal_inferior (struct target_ops *self) @@ -348,7 +352,10 @@ child_terminal_inferior (struct target_ops *self) so that no input is discarded. After doing this, either terminal_ours or terminal_inferior - should be called to get back to a normal state of affairs. */ + should be called to get back to a normal state of affairs. + + N.B. The implementation is (currently) no different than + child_terminal_ours. See child_terminal_ours_1. */ void child_terminal_ours_for_output (struct target_ops *self) @@ -358,7 +365,11 @@ child_terminal_ours_for_output (struct target_ops *self) /* Put our terminal settings into effect. First record the inferior's terminal settings - so they can be restored properly later. */ + so they can be restored properly later. + + N.B. Targets that want to use this with async support must build that + support on top of this (e.g., the caller still needs to add stdin to the + event loop). E.g., see linux_nat_terminal_ours. */ void child_terminal_ours (struct target_ops *self) diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 8d4251f..5a791bc 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -4595,7 +4595,9 @@ linux_nat_supports_disable_randomization (struct target_ops *self) static int async_terminal_is_ours = 1; -/* target_terminal_inferior implementation. */ +/* target_terminal_inferior implementation. + + This is a wrapper around child_terminal_inferior to add async support. */ static void linux_nat_terminal_inferior (struct target_ops *self) @@ -4618,7 +4620,14 @@ linux_nat_terminal_inferior (struct target_ops *self) set_sigint_trap (); } -/* target_terminal_ours implementation. */ +/* target_terminal_ours implementation. + + This is a wrapper around child_terminal_ours to add async support (and + implement the target_terminal_ours vs target_terminal_ours_for_output + distinction). child_terminal_ours is currently no different than + child_terminal_ours_for_output. + We leave target_terminal_ours_for_output alone, leaving it to + child_terminal_ours_for_output. */ static void linux_nat_terminal_ours (struct target_ops *self)