From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36330 invoked by alias); 21 Dec 2017 12:55:59 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 36312 invoked by uid 89); 21 Dec 2017 12:55:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-pg0-f44.google.com Received: from mail-pg0-f44.google.com (HELO mail-pg0-f44.google.com) (74.125.83.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 21 Dec 2017 12:55:57 +0000 Received: by mail-pg0-f44.google.com with SMTP id m25so13193612pgv.12 for ; Thu, 21 Dec 2017 04:55:56 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=5R0A+ZGe1lD1z+THe1Na4gZDvS1FjmkBZsPmPATPtbo=; b=qJy9tN2JMe0IZXbnWZVuz4Os3LIM+JhXSA7IGtLhOaUx3hi3uKByeQwqYFx8TObYWB UxwSFlKhDLM4xU+fZivs2V41jVdEFeSKt/idPF6fENu2qM9nAg0ZhqYJGh/XVWnv2jm9 T8KFGDsvDoZMrcHlXd+M06GiN/Dcn1ju/835uVvtW7Ai0GxavkqLQ/6T5NjSHzJrAcHg 9ZoCPE2VdDQKTjNbj/GT+xbbysFgc9lrBfFgaajn1aGoRPeWN9YV0D8qqyfwJSRPEUtR 7j/D5RRqQlC8lkGNjQYjvm9D+icBvLQZBL9n8LZL0zv0StyX1IdR3byahxP2YQgx0nPA 4/WQ== X-Gm-Message-State: AKGB3mJPejflrb8pe76CnHpmmC3GiRff6tQlzTw0/UPpS6pf3x7kMPNu 6Hs0NnSFsLciNkD678vG+Fo= X-Google-Smtp-Source: ACJfBovgtKokD0kw4obQ/yX8e0sgmkKh9CfsCVRwWH7i1Mq8nabRyQhrSbjeoqrAkdf0bx0NmFpvrw== X-Received: by 10.99.129.73 with SMTP id t70mr9649073pgd.424.1513860955143; Thu, 21 Dec 2017 04:55:55 -0800 (PST) Received: from localhost (g128.222-224-187.ppp.wakwak.ne.jp. [222.224.187.128]) by smtp.gmail.com with ESMTPSA id q68sm38003412pfb.1.2017.12.21.04.55.54 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 21 Dec 2017 04:55:54 -0800 (PST) Date: Thu, 21 Dec 2017 12:55:00 -0000 From: Stafford Horne To: Simon Marchi Cc: GDB patches , Openrisc Subject: Re: [PATCH v3 4/4] tdesc: handle arbitrary strings in tdesc_register_in_reggroup_p Message-ID: <20171221125552.GE32243@lianli.shorne-pla.net> References: <20171219142257.13402-1-shorne@gmail.com> <20171219142257.13402-5-shorne@gmail.com> <842bc3de1ad8cd8a568127aedeb58275@polymtl.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <842bc3de1ad8cd8a568127aedeb58275@polymtl.ca> User-Agent: Mutt/1.9.1 (2017-09-22) X-IsSubscribed: yes X-SW-Source: 2017-12/txt/msg00471.txt.bz2 On Wed, Dec 20, 2017 at 10:29:31PM -0500, Simon Marchi wrote: > On 2017-12-19 09:22, Stafford Horne wrote: > > @@ -1299,26 +1294,9 @@ tdesc_register_in_reggroup_p (struct gdbarch > > *gdbarch, int regno, > > { > > struct tdesc_reg *reg = tdesc_find_register (gdbarch, regno); > > > > - if (reg != NULL && !reg->group.empty ()) > > - { > > - int general_p = 0, float_p = 0, vector_p = 0; > > - > > - if (reg->group == "general") > > - general_p = 1; > > - else if (reg->group == "float") > > - float_p = 1; > > - else if (reg->group == "vector") > > - vector_p = 1; > > - > > - if (reggroup == float_reggroup) > > - return float_p; > > - > > - if (reggroup == vector_reggroup) > > - return vector_p; > > - > > - if (reggroup == general_reggroup) > > - return general_p; > > - } > > + if (reg != NULL && !reg->group.empty () > > + && (strcmp (reg->group.c_str (), reggroup_name (reggroup)) == 0)) > > I suggest > > reg->group == reggroup_name (reggroup) OK. Strange, I thought I tried that first but was having issues with std::string vs (char *), but I just tried again and it works fine. > > + return 1; > > > > if (reg != NULL > > && (reggroup == save_reggroup || reggroup == restore_reggroup)) > > @@ -1421,6 +1399,28 @@ tdesc_use_registers (struct gdbarch *gdbarch, > > void **slot = htab_find_slot (reg_hash, reg.get (), INSERT); > > > > *slot = reg.get (); > > + /* Add reggroup if its new. */ > > + if (!reg->group.empty ()) > > + { > > + struct reggroup *group; > > + bool group_exists = false; > > + > > + for (group = reggroup_next (gdbarch, NULL); > > + group != NULL; > > + group = reggroup_next (gdbarch, group)) > > + { > > + if (strcmp (reg->group.c_str (), reggroup_name (group)) == 0) > > Here too. OK. > > + { > > + group_exists = true; > > + break; > > + } > > + } > > + > > + if (!group_exists) > > + reggroup_add (gdbarch, reggroup_gdbarch_new (gdbarch, > > + reg->group.c_str (), > > + USER_REGGROUP)); > > + } > > Could you factor this out in a separate function? It would probably be > useful to have a general-purpose function > > reggroup *reggroup_find (struct gdbarch *gdbarch, const char *name); > > Which you could then use here > > if (reggroup_find (...) == NULL) > { > // Create group > } Sure good point, if I find any places I can use it I will. Thank You, -Stafford