From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id E57463857B95 for ; Mon, 18 Dec 2023 15:42:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E57463857B95 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E57463857B95 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702914172; cv=none; b=kcShRyuUnnboiTiHioz+3KYs0zcKdFiCUndQNti+RGEUBKN8i/2reMuJiMZNHluBLvrWVmL0uLT7IDLpa9vH/Wv6BulGnX4OT0uElLgjHXoWY+K9BanL5SM7wcTdxrChai8yZ8gLW6ksJr92Ma7m6kU+B1jfpjWC93le4PUtkUY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702914172; c=relaxed/simple; bh=6v01aMzqS1r25bUK/VkS1EKg9DwImBx2oZTZUMaZU24=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=jvEnAn3qmovPq4AZ+v9PTDRWV9X89GJ4gy175ciKKmQ4DPlL60OzqapNuWgXPr7DNVPwKNztQyL4V9c5jk52NUrvGtJMJ3oDPI70OhGzer+aRPEsdbUybKT4CWHahSnRWBQEU1DEERKvaR6n37hbudrw0NsHxkKEXtmONYFNlUw= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1702914170; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=09hl1rOiu9sbsiSNV4p7vmuWOrg+NMwikXaiW0EZcy0=; b=SbQD3s94PTcSMNuQj5DIybFaL5QWopYWO8pz0lKIQr7BAHlmHvboBf+ZHb1kUB3/O9t6ma RtxlJz1TfOrDUFT+UMfo2Cbl13sWpXgUgt/1L8FZKQvfXO74epSWemxQH8DkZ2VbO3b2OM OGWYzxOhMwlLu500BPX5FLweQL5tmzs= Received: from mail-lf1-f72.google.com (mail-lf1-f72.google.com [209.85.167.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-156-V6sqDliMME--4tdy8V7tnA-1; Mon, 18 Dec 2023 10:42:49 -0500 X-MC-Unique: V6sqDliMME--4tdy8V7tnA-1 Received: by mail-lf1-f72.google.com with SMTP id 2adb3069b0e04-50bfda09704so2777256e87.1 for ; Mon, 18 Dec 2023 07:42:49 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702914167; x=1703518967; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=09hl1rOiu9sbsiSNV4p7vmuWOrg+NMwikXaiW0EZcy0=; b=Ns70BylEgWU6K6d5KxoLjNw2gtbfyqrrMYndl5dUMXz17icjCqlj6DtqTWWSwUpl3O nz37HnDrCqADzz9fhycBVak/4cyCk0srufFXuZUHcn1/O8rKIc3oqXUijp0B34BGCCmm svjgPWZf7sBect1tPsaUptvk9gIc1i6yeNcNcxJ2VurD7s1IwmJl2lklbcMth9QDWNt/ +/R1NNnK5120jMqmm8HMoZlJ6aZJ/Q0X2vEP9VmUv9bPSYA08+lWET9VGECkSOrcJcoq /B4/ziiIBzDwvzcg6/BD7R9BvPHv1Dl1gYX9qlkiOaE4SQbZGwQ25usgb+D6Enc3orMD d1kg== X-Gm-Message-State: AOJu0YzwnWrJM+A6xlsOHso6kFcZUDikfTyUhfVB/1xRnBwve5qSVmd9 LhggIgOEZqgrYSGav+3HslOYBc9s7W1rZkhoej2kSXc18PIj7u4xTtOklmkaSyaq2SzX7mN5QTX 7NjIXyCThiuRTthmCH05sK1QB4G1KgQ== X-Received: by 2002:ac2:4a63:0:b0:50e:2d18:9b86 with SMTP id q3-20020ac24a63000000b0050e2d189b86mr1570643lfp.88.1702914167643; Mon, 18 Dec 2023 07:42:47 -0800 (PST) X-Google-Smtp-Source: AGHT+IFBXPscR9R7kt+nJunjz9hq0sC/4h1CcmohFZr3esb/FRfOjQOnDT7RWJGr69UVkb1Hk1CkSg== X-Received: by 2002:ac2:4a63:0:b0:50e:2d18:9b86 with SMTP id q3-20020ac24a63000000b0050e2d189b86mr1570625lfp.88.1702914167112; Mon, 18 Dec 2023 07:42:47 -0800 (PST) Received: from localhost (105.226.159.143.dyn.plus.net. [143.159.226.105]) by smtp.gmail.com with ESMTPSA id kq26-20020a170906abda00b00a236e367766sm59932ejb.23.2023.12.18.07.42.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 Dec 2023 07:42:46 -0800 (PST) From: Andrew Burgess To: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [PATCH 03/14] Simplify tui_data_window::show_register_group In-Reply-To: <20231217-tui-regs-cleanup-v1-3-67bd0ea1e8be@tromey.com> References: <20231217-tui-regs-cleanup-v1-0-67bd0ea1e8be@tromey.com> <20231217-tui-regs-cleanup-v1-3-67bd0ea1e8be@tromey.com> Date: Mon, 18 Dec 2023 15:42:46 +0000 Message-ID: <87msu74ivd.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,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: Tom Tromey writes: > This simplifies tui_data_window::show_register_group, renaming it as > well. The old method had two loops to iterate over all the registers > for the arch, but in the new scheme, the vector is set up when > switching groups, and then updates simply iterate over the vector. > > tui_data_item_window is made self-updating, which also clarifies the > code somewhat. > --- > gdb/tui/tui-regs.c | 104 ++++++++++++++++++++--------------------------------- > gdb/tui/tui-regs.h | 16 ++++++--- > 2 files changed, 49 insertions(+), 71 deletions(-) > > diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c > index 91fbf75c250..7b8bf323f50 100644 > --- a/gdb/tui/tui-regs.c > +++ b/gdb/tui/tui-regs.c > @@ -104,21 +104,17 @@ tui_register_format (frame_info_ptr frame, int regnum) > /* Get the register value from the given frame and format it for the > display. When changedp is set, check if the new register value has > changed with respect to the previous call. */ > -static void > -tui_get_register (frame_info_ptr frame, > - struct tui_data_item_window *data, > - int regnum, bool *changedp) > +void > +tui_data_item_window::update (const frame_info_ptr &frame) The comment on this function needs updating. > { > - if (changedp) > - *changedp = false; > if (target_has_registers ()) > { > - std::string new_content = tui_register_format (frame, regnum); > + std::string new_content = tui_register_format (frame, regno); > > - if (changedp != NULL && data->content != new_content) > - *changedp = true; > + if (content != new_content) > + highlight = true; > > - data->content = std::move (new_content); > + content = std::move (new_content); > } > } > > @@ -178,13 +174,11 @@ tui_data_window::show_registers (const reggroup *group) > > if (target_has_registers () && target_has_stack () && target_has_memory ()) > { > - show_register_group (group, get_selected_frame (NULL), > - group == m_current_group); > + update_register_data (group, get_selected_frame (NULL)); Maybe s/NULL/nullptr/ ? > > /* Clear all notation of changed values. */ > for (auto &&data_item_win : m_regs_content) > data_item_win.highlight = false; > - m_current_group = group; > } > else > { > @@ -201,63 +195,43 @@ tui_data_window::show_registers (const reggroup *group) > refresh_values_only is true. */ > > void > -tui_data_window::show_register_group (const reggroup *group, > - frame_info_ptr frame, > - bool refresh_values_only) > +tui_data_window::update_register_data (const reggroup *group, > + frame_info_ptr frame) The comment on this function needs updating. > { > - struct gdbarch *gdbarch = get_frame_arch (frame); > - int nr_regs; > - int regnum, pos; > - > - /* Make a new title showing which group we display. */ > - this->set_title (string_printf ("Register group: %s", group->name ())); > - > - /* See how many registers must be displayed. */ > - nr_regs = 0; > - for (regnum = 0; regnum < gdbarch_num_cooked_regs (gdbarch); regnum++) > + if (group != m_current_group) > { > - const char *name; > - > - /* Must be in the group. */ > - if (!gdbarch_register_reggroup_p (gdbarch, regnum, group)) > - continue; > - > - /* If the register name is empty, it is undefined for this > - processor, so don't display anything. */ > - name = gdbarch_register_name (gdbarch, regnum); > - if (*name == '\0') > - continue; > - > - nr_regs++; > - } > + m_current_group = group; > > - m_regs_content.resize (nr_regs); > + /* Make a new title showing which group we display. */ > + this->set_title (string_printf ("Register group: %s", group->name ())); > > - /* Now set the register names and values. */ > - pos = 0; > - for (regnum = 0; regnum < gdbarch_num_cooked_regs (gdbarch); regnum++) > - { > - struct tui_data_item_window *data_item_win; > - const char *name; > + /* Create the registers. */ > + m_regs_content.clear (); > > - /* Must be in the group. */ > - if (!gdbarch_register_reggroup_p (gdbarch, regnum, group)) > - continue; > + struct gdbarch *gdbarch = get_frame_arch (frame); > + for (int regnum = 0; > + regnum < gdbarch_num_cooked_regs (gdbarch); > + regnum++) > + { > + /* Must be in the group. */ > + if (!gdbarch_register_reggroup_p (gdbarch, regnum, group)) > + continue; > > - /* If the register name is empty, it is undefined for this > - processor, so don't display anything. */ > - name = gdbarch_register_name (gdbarch, regnum); > - if (*name == '\0') > - continue; > + /* If the register name is empty, it is undefined for this > + processor, so don't display anything. */ > + const char *name = gdbarch_register_name (gdbarch, regnum); > + if (*name == '\0') > + continue; > > - data_item_win = &m_regs_content[pos]; > - if (!refresh_values_only) > - { > - data_item_win->regno = regnum; > - data_item_win->highlight = false; > + m_regs_content.emplace_back (regnum, frame); > } > - tui_get_register (frame, data_item_win, regnum, 0); > - pos++; > + } > + else > + { > + /* The group did not change, so we can simply update each > + item. */ > + for (tui_data_item_window ® : m_regs_content) > + reg.update (frame); > } > } > > @@ -470,13 +444,11 @@ tui_data_window::check_register_values (frame_info_ptr frame) > show_registers (m_current_group); > else > { > - for (auto &&data_item_win : m_regs_content) > + for (tui_data_item_window &data_item_win : m_regs_content) > { > bool was_hilighted = data_item_win.highlight; > > - tui_get_register (frame, &data_item_win, > - data_item_win.regno, > - &data_item_win.highlight); > + data_item_win.update (frame); > > /* Register windows whose y == 0 are outside the visible area. */ > if ((data_item_win.highlight || was_hilighted) > diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h > index 1abd22cd382..9c451ba5996 100644 > --- a/gdb/tui/tui-regs.h > +++ b/gdb/tui/tui-regs.h > @@ -29,19 +29,26 @@ > > struct tui_data_item_window > { > - tui_data_item_window () = default; > + tui_data_item_window (int regno, const frame_info_ptr &frame) > + : regno (regno) > + { > + update (frame); > + highlight = false; > + } > > DISABLE_COPY_AND_ASSIGN (tui_data_item_window); > > tui_data_item_window (tui_data_item_window &&) = default; > > + void update (const frame_info_ptr &frame); > + > void rerender (WINDOW *handle, int field_width); > > /* Location. */ > int x = 0; > int y = 0; > /* The register number. */ > - int regno = -1; > + int regno; If we're touching this anyway, maybe we could take the opportunity to make this private? Thanks, Andrew > bool highlight = false; > bool visible = false; > std::string content; > @@ -104,9 +111,8 @@ struct tui_data_window : public tui_win_info > display off the end of the register display. */ > void display_reg_element_at_line (int start_element_no, int start_line_no); > > - void show_register_group (const reggroup *group, > - frame_info_ptr frame, > - bool refresh_values_only); > + void update_register_data (const reggroup *group, > + frame_info_ptr frame); > > /* Answer the number of the last line in the regs display. If there > are no registers (-1) is returned. */ > > -- > 2.43.0