From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id D80E3385841B for ; Sun, 31 Oct 2021 02:43:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D80E3385841B 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 19V2h7ZL027570 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 30 Oct 2021 22:43:12 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 19V2h7ZL027570 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 288351ECEB; Sat, 30 Oct 2021 22:43:07 -0400 (EDT) Message-ID: <5f8f0251-99a9-d4f2-69ae-fe2ad68c838f@polymtl.ca> Date: Sat, 30 Oct 2021 22:43:06 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.1 Subject: Re: [PATCH 1/2] gdb: add set/show commands for managing debuginfod Content-Language: en-US To: Aaron Merey Cc: gdb-patches@sourceware.org References: <20211028221823.574570-1-amerey@redhat.com> <6137615d-9b33-8e24-4d81-5aa998d7fc1b@polymtl.ca> From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sun, 31 Oct 2021 02:43:07 +0000 X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org 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: , X-List-Received-Date: Sun, 31 Oct 2021 02:43:16 -0000 On 2021-10-29 21:54, Simon Marchi via Gdb-patches wrote: > On 2021-10-29 21:09, Aaron Merey wrote: >> Changed to use add_setshow_prefix_cmd. Because this function >> doesn't have a parameter for a custom show function I added >> a 'show debuginfod status' command so that the user can check >> whether debuginfod is set to 'on', 'off' or 'ask'. > > Ah, I had not realized that, sorry. Well, you could have left it > as-is then :/. Hi Aaron, I re-visited how I suggested you to do "set debuginfod", which were based on how I designed "set index-cache", and I think it's not right after all. I don't think it was good idea to overload "set index-cache" as a boolean setting and a prefix command. First, I had to add the information of whether index-cache is enabled as custom message: (gdb) show index-cache index-cache directory: The directory of the index cache is "/home/simark/.cache/gdb". index-cache stats: Cache hits (this session): 0 Cache misses (this session): 0 The index cache is currently disabled. <--- This is not good, because in "info set" it looks like this: (gdb) info set ... host-charset: The host character set is "auto; currently UTF-8". index-cache directory: The directory of the index cache is "/home/simark/.cache/gdb". index-cache stats: Cache hits (this session): 0 Cache misses (this session): 0 inferior-tty: Terminal for future runs of program being debugged is "". ... This just looks weird. Also, because the index-cache on/off knob isn't a proper setting, it doesn't appear in MI, a frontend can't know if index-cache is enabled or not: -gdb-show index-cache ~"\n" ~" Cache hits (this session): 0\n" ~" Cache misses (this session): 0\n" ~"\n" ~"The index cache is currently disabled.\n" ^done,showlist={option={name="directory",value="/home/simark/.cache/gdb"},option={name="stats"}} You solved this by adding "set debuginfod status", which is a bit better, but also not ideal. It's weird for the set command used to toggle debuginfod on and off to not mirror the show command used to show the state. So, I think that having dedicated commands "set/show index-cache enabled on/off" and "set/show debuginfod enabled on/off" is better, in the end. What do you think? Another mistake I think I did with the index-cache is "show index-cache stats". Set/show commands are supposed to be for settings. This is just printing some information, not a setting value. Using a show command for this means that the index-cache stats are printed when doing "info set", which makes no sense. Also, printing the "setting" in MI is useless: -gdb-show index-cache stats ~" Cache hits (this session): 0\n" ~"Cache misses (this session): 0\n" ^done I think that this should have been an info command, "info index-cache stats" maybe. I'm considering fixing this for the index-cache (deprecating the old commands, but keeping them as aliases of the new ones). If you agree, I could do the same with "set debuginfo" too. Simon