From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id sWp3NkQvEGGAKgAAWB0awg (envelope-from ) for ; Sun, 08 Aug 2021 15:23:48 -0400 Received: by simark.ca (Postfix, from userid 112) id CD0931EDF7; Sun, 8 Aug 2021 15:23:48 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.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 813301E79C for ; Sun, 8 Aug 2021 15:23:47 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BF2533861032 for ; Sun, 8 Aug 2021 19:23:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BF2533861032 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1628450625; bh=RTradA/i6e2lLGVa6v4QxESCtSPUT7hhWSuyeBhI8Us=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=hgIGcurWM3Mw2aFoV/wICoEt9MNWVvzSKgH+wgyA9F2UteFa2SJ6lVKSF5Xwzh4Nr 1tf0bFNRMUWwBn9QBQAe4xM23qqusA2GWQ9m+EBzv7sVV31cYMkLmBKbOs8nFHffsr Sraaj1Oy4TwrmRgZw/3JHtlmauHkdw+s5MiMtYS8= Received: from lndn.lancelotsix.com (vps-42846194.vps.ovh.net [IPv6:2001:41d0:801:2000::2400]) by sourceware.org (Postfix) with ESMTPS id 10ECD385DC30 for ; Sun, 8 Aug 2021 19:23:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 10ECD385DC30 Received: from Plymouth.lan (unknown [IPv6:2a02:390:9086:0:634a:c6a1:40ee:68b1]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id F010481882; Sun, 8 Aug 2021 19:23:21 +0000 (UTC) To: gdb-patches@sourceware.org Subject: [PATCH v2 0/4] gdb: Refactor cmd_list_element.var Date: Sun, 8 Aug 2021 20:22:58 +0100 Message-Id: <20210808192302.3768766-1-lsix@lancelotsix.com> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Sun, 08 Aug 2021 19:23:22 +0000 (UTC) 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: Lancelot SIX via Gdb-patches Reply-To: Lancelot SIX Cc: Lancelot SIX , Tom Tromey Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" Hi, This is a V2 for https://sourceware.org/pipermail/gdb-patches/2021-July/181183.html Noticeable changes since V1: - Based on the feedbacks form Tom Tromey, the API have been simplified and the amount of templated code have been reduced. Instead of using something like setting.get () in order to access the value of a setting backed by an int (which must be either a var_integer, a var_zinetger or a var_zuinteger_unlimited), we now write setting.get () The 'get' function still includes an assert that setting.var_type is one of the var_types that is backed by an int. In this version, the call site needs to know the mapping between var_types and underlying storage type. - Based on a comment from Simon Marchi, the name of the type that ties together the var_type and the void * pointer has changed from 'param' to 'setting'. - Minor formatting issues have been addressed. --- This patch series proposes to refactor the way cmd_list_element.var is handled. It is meant to be a cleanup series and is intended to offer a basis on which to solve PR/28085. Some commands contain a reference to a variable (setting) that can be set or shown. To do that, the command element contains pieces of information: a void* pointer to global variable, and a var_types field to indicate the type of the setting. The var_type dictates what the data type of variable pointed by the void* pointer must be. When any piece of code within GDB tries to interact with such variable, it first checks the type field and then casts the void* pointer into whatever type pointer it needs to and dereferences it. There are few limitations with this approach I try to address in this patch series: - Any code that interact with a cmd_list_element.var is responsible to do some cast to the appropriate type. This is done in multiple places in the codebase and is quite easy to get wrong. The first patch tries to cover that by introducing a safer abstraction around the void * pointer. The accessors that hide the void * pointer adds runtime checks that the pointer is casted to the appropriate type that matches the var_type of the setting instance. - The source of truth for any setting needs to be the variable pointed to. However, as pointed out by Simon in PR/28085 this is not always true, and there are bugs linked to this. In order to solve this problem, patch 3 allows getter and setter callbacks to be embedded in the abstraction introduced in patch 1. When the setting is accessed, the callback is called to compute or update the value based on the current state of GDB. Patch 2, which was originally proposed by Simon Marchi[1] proposes to change the variable type from char* to std::string for string like setting. Finally Patch 4 does some more refactoring on how we detect and notify observers when a given setting is updated. None of those patches should introduce user visible changes. The entire series have been tested on x86_64 and no regression have been observed on the testsuite. All feedbacks are welcome. Lancelot SIX (3): gdb: Add typesafe getter/setter for cmd_list_element.var gdb: Have setter and getter callbacks for settings gdb: Setting setter return a bool to tell if the value changed Simon Marchi (1): gdb: make string-like set show commands use std::string variable gdb/auto-load.c | 50 ++--- gdb/breakpoint.c | 22 +- gdb/build-id.c | 4 +- gdb/cli/cli-cmds.c | 80 ++++--- gdb/cli/cli-decode.c | 425 +++++++++++++++++++++++++++++------ gdb/cli/cli-decode.h | 6 +- gdb/cli/cli-logging.c | 23 +- gdb/cli/cli-option.c | 9 +- gdb/cli/cli-option.h | 4 +- gdb/cli/cli-setshow.c | 188 +++++++--------- gdb/command.h | 419 +++++++++++++++++++++++++++++++++- gdb/compile/compile.c | 46 ++-- gdb/corefile.c | 17 +- gdb/defs.h | 4 +- gdb/disasm.c | 11 +- gdb/disasm.h | 2 +- gdb/dwarf2/dwz.c | 2 +- gdb/dwarf2/index-cache.c | 10 +- gdb/dwarf2/read.c | 10 +- gdb/event-top.c | 12 +- gdb/fork-child.c | 7 +- gdb/guile/scm-param.c | 163 ++++++++------ gdb/infcmd.c | 14 +- gdb/linux-thread-db.c | 17 +- gdb/main.c | 17 +- gdb/maint-test-options.c | 11 +- gdb/maint-test-settings.c | 8 +- gdb/maint.c | 2 +- gdb/mi/mi-cmd-env.c | 18 +- gdb/proc-api.c | 5 +- gdb/python/py-param.c | 58 +++-- gdb/python/python-internal.h | 2 +- gdb/python/python.c | 31 +-- gdb/remote-sim.c | 7 +- gdb/remote.c | 12 +- gdb/serial.c | 8 +- gdb/solib.c | 20 +- gdb/source.c | 66 +++--- gdb/source.h | 5 +- gdb/stack.c | 22 +- gdb/symfile.c | 49 ++-- gdb/symtab.c | 46 ++-- gdb/target-descriptions.c | 2 +- gdb/top.c | 112 +++++---- gdb/top.h | 2 +- gdb/tracepoint.c | 29 +-- gdb/tracepoint.h | 2 +- 47 files changed, 1378 insertions(+), 701 deletions(-) -- 2.31.1