From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 42wbBGkjUGAcTwAAWB0awg (envelope-from ) for ; Mon, 15 Mar 2021 23:18:01 -0400 Received: by simark.ca (Postfix, from userid 112) id 029EA1EF78; Mon, 15 Mar 2021 23:18:01 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 23B421E789 for ; Mon, 15 Mar 2021 23:18:00 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 67CB63858C27; Tue, 16 Mar 2021 03:17:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 67CB63858C27 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1615864679; bh=bB7cW1vmjhn4gUYnQwlBu2E3WhUEjEgUgF0utacZPnI=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=TnHOQA9mL1hnXTliXeUsNtHJjWrfyZ5OxTa+QiCqdFc10REgWdXPPYgm1A6E/CDzP 07aSsq8rdS+XLHzRExZY9jr+xTFchooB9z0c1fDbVH14k8UdNI1gyLbpP2ZqxTeJx/ cFK+RnbhiIdsXU4Vu9Kcvg5k4F1jAfLDKMfGaE2s= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 3A7FD3858C27 for ; Tue, 16 Mar 2021 03:17:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3A7FD3858C27 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 12G3Hn5h031659 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 15 Mar 2021 23:17:53 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 12G3Hn5h031659 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 0C8F31E789; Mon, 15 Mar 2021 23:17:48 -0400 (EDT) Subject: Re: [PATCH v2 2/5] gdb: prevent prefix commands from being hooks To: Marco Barisione References: <20210108100706.96190-1-mbarisione@undo.io> <20210125112649.56362-1-mbarisione@undo.io> <20210125112649.56362-3-mbarisione@undo.io> <1f3b725d-4b0a-d9ab-7553-24b52b3ff256@polymtl.ca> Message-ID: Date: Mon, 15 Mar 2021 23:17:48 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Tue, 16 Mar 2021 03:17:49 +0000 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Cc: GDB patches mailing list Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 2021-03-09 4:42 a.m., Marco Barisione wrote: > On 8 Mar 2021, at 21:32, Simon Marchi wrote: >> Ahhh, changes that touch command handling are always scary stuff, we >> need to be really careful not to break existing use cases or paint >> ourselves in corners for future use cases. Probably why nobody dares to >> review it. I'll give it a try, but know that it still scares me :). > > It did scare me and took a while because of the many corner cases, > difficult combinations of features. > > My reasoning behind this is that the behaviour is currently confusing > (that is, what the user would expect doesn’t happen) and forbidding > this should allow changes in the future quite easily. > >> One question that pops up is, what if someone ever wants to make a hook >> for a prefixed command. For example, let's say I want to hook "info >> inferiors". I don't think that's possible today, but let's say we want >> to make it possible. How would that work? >> >> This doesn't work: >> >> (gdb) define hook-info inferiors >> Undefined command: "hook-info". Try "help". >> >> So... maybe defining "hook-info" as a prefix command would allow it? >> Not sure, I didn't really think this through, but I wonder if you >> thought about that case. > > You can do it with “define info hook-inferiors” which I found a bit > surprising when I discovered it. I didn't know that! And it's even tested: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/testsuite/gdb.base/define.exp;h=ff34e7f294376454fbefb868f4e3f90b6105f0d8;hb=HEAD#l296 ... and documented in the manual. > I think that hooks are incredibly useful but limited. My hope is that > my patch series will make them less needed as you can get a better and > more powerful behaviour with command renaming. I mentioned in a previous review (somewhere in the default-args series, I can't find it right now) that I was not too comfortable in having features allowing modifying the behavior of builtin commands in undocumented ways. You could say that hooks are a way to do that. But at least, even if you define a hook-backtrace, the backtrace part still works as usual, just that there are some things happening before. Maybe I'm a bit too afraid that some script I'm using breaks some other script I'm using because it decided to rename some core command... But I see why that would be useful (we use proc renaming in TCL for the testsuite). It's a tradeoff, with powerful tools you can go great things but are also more likely to shoot yourself in the foot. And I guess we can't protect the users from themselves for everything, otherwise our tool is useless. And since the concept was deemed ok by other reviewers / maintainers previously, I won't object to it. Simon