From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 114541 invoked by alias); 7 Jun 2018 20:56:02 -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 114525 invoked by uid 89); 7 Jun 2018 20:56:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3 autolearn=ham version=3.3.2 spammy=seeing, raw_compare X-HELO: sessmg22.ericsson.net Received: from sessmg22.ericsson.net (HELO sessmg22.ericsson.net) (193.180.251.58) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 07 Jun 2018 20:55:59 +0000 Received: from ESESSHC009.ericsson.se (Unknown_Domain [153.88.183.45]) by sessmg22.ericsson.net (Symantec Mail Security) with SMTP id 0C.B7.24558.DDB991B5; Thu, 7 Jun 2018 22:55:57 +0200 (CEST) Received: from ESESSMB501.ericsson.se (153.88.183.162) by ESESSHC009.ericsson.se (153.88.183.45) with Microsoft SMTP Server (TLS) id 14.3.382.0; Thu, 7 Jun 2018 22:55:56 +0200 Received: from ESESBMB505.ericsson.se (153.88.183.172) by ESESSMB501.ericsson.se (153.88.183.162) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3; Thu, 7 Jun 2018 22:55:56 +0200 Received: from NAM03-DM3-obe.outbound.protection.outlook.com (153.88.183.157) by ESESBMB505.ericsson.se (153.88.183.172) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3 via Frontend Transport; Thu, 7 Jun 2018 22:55:56 +0200 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; Received: from [142.133.60.240] (192.75.88.130) by DM6PR15MB2396.namprd15.prod.outlook.com (2603:10b6:5:8d::30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.841.14; Thu, 7 Jun 2018 20:55:54 +0000 Subject: Re: [PATCH v2 04/10] Add regcache raw_compare method To: Alan Hayward , CC: References: <20180606151629.36602-1-alan.hayward@arm.com> <20180606151629.36602-5-alan.hayward@arm.com> From: Simon Marchi Message-ID: Date: Thu, 07 Jun 2018 20:56:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180606151629.36602-5-alan.hayward@arm.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-ClientProxiedBy: SN4PR0501CA0010.namprd05.prod.outlook.com (2603:10b6:803:40::23) To DM6PR15MB2396.namprd15.prod.outlook.com (2603:10b6:5:8d::30) X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DM6PR15MB2396: X-Exchange-Antispam-Report-Test: UriScan:(180628864354917); X-MS-Exchange-SenderADCheck: 1 X-Forefront-PRVS: 06968FD8C4 Received-SPF: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-MS-Office365-Filtering-Correlation-Id: 243b5a87-aa5b-40c2-6a8f-08d5ccb90fa7 X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Jun 2018 20:55:54.2064 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 243b5a87-aa5b-40c2-6a8f-08d5ccb90fa7 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 92e84ceb-fbfd-47ab-be52-080c6b87953f X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR15MB2396 X-OriginatorOrg: ericsson.com X-IsSubscribed: yes X-SW-Source: 2018-06/txt/msg00183.txt.bz2 On 2018-06-06 11:16 AM, Alan Hayward wrote: > raw_compare returns the same as a memcmp (0 for success, > the diff otherwise). Kept with tristate return as this > feels potentally more useful than a simple true/false return. > (Happy to change if not). I would err on the bool side, but I don't really mind, the important is that it's documented properly. We still use many ints as bool in GDB, so someone seeing an int return value could easily think it's 0 -> not equal, 1 -> equal. Speaking of doc, I would suggest (as in the previous patch) to centralize the doc in the class/struct at the root, so we don't duplicate it. > 2018-06-06 Alan Hayward > > gdb/ > * common/common-regcache.h (raw_compare): New function. > * regcache.c (regcache::raw_compare): Likewise. > * regcache.h (regcache::raw_compare): New declaration. > > gdbserver/ > * regcache.c (regcache::raw_compare): New function. > * regcache.h (regcache::raw_compare): New declaration. > --- > gdb/common/common-regcache.h | 1 + > gdb/gdbserver/regcache.c | 10 ++++++++++ > gdb/gdbserver/regcache.h | 5 +++++ > gdb/regcache.c | 15 +++++++++++++++ > gdb/regcache.h | 11 +++++++++++ > 5 files changed, 42 insertions(+) > > diff --git a/gdb/common/common-regcache.h b/gdb/common/common-regcache.h > index 487da0a7fb..e91439bec5 100644 > --- a/gdb/common/common-regcache.h > +++ b/gdb/common/common-regcache.h > @@ -67,6 +67,7 @@ struct reg_buffer_common > virtual ~reg_buffer_common () = default; > virtual void raw_supply (int regnum, const void *buf) = 0; > virtual void raw_collect (int regnum, void *buf) const = 0; > + virtual int raw_compare (int regnum, const void *buf, int offset) const = 0; > virtual register_status get_register_status (int regnum) const = 0; > }; > > diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c > index 857721ee3d..9648428349 100644 > --- a/gdb/gdbserver/regcache.c > +++ b/gdb/gdbserver/regcache.c > @@ -501,3 +501,13 @@ regcache::get_register_status (int regnum) const > return REG_VALID; > #endif > } > + > +/* See gdbserver/regcache.h. */ > + > +int > +regcache::raw_compare (int regnum, const void *buf, int offset) const > +{ > + gdb_assert (register_size (tdesc, regnum) > offset); Should we check ">= offset"? I think it could be useful for some edge cases where we could avoid an "if (offset < size)" in the caller, if we know offset could be equal to size. memcmp would return 0 (equal), which is fine. Maybe assign "register_size (tdesc, regnum)" to a variable (e.g. reg_size) and use it in both places? > + return memcmp (buf, register_data (this, regnum, 1) + offset, > + register_size (tdesc, regnum) - offset); > +} > diff --git a/gdb/gdbserver/regcache.h b/gdb/gdbserver/regcache.h > index 1842f1d9cf..b26f39a8ad 100644 > --- a/gdb/gdbserver/regcache.h > +++ b/gdb/gdbserver/regcache.h > @@ -50,6 +50,11 @@ struct regcache : public reg_buffer_common > > void raw_collect (int regnum, void *buf) const override; > > + /* Compare the contents of the register stored in the regcache (ignoring the > + first OFFSET bytes) to the contents of BUF (without any offset). Returns > + the same result as memcmp. */ > + int raw_compare (int regnum, const void *buf, int offset) const override; > + > enum register_status get_register_status (int regnum) const override; > }; > > diff --git a/gdb/regcache.c b/gdb/regcache.c > index a914b548ca..383e355e9f 100644 > --- a/gdb/regcache.c > +++ b/gdb/regcache.c > @@ -1082,6 +1082,21 @@ regcache::collect_regset (const struct regset *regset, > transfer_regset (regset, NULL, regnum, NULL, buf, size); > } > > +/* See regcache.h. */ > + > +int > +regcache::raw_compare (int regnum, const void *buf, int offset) const > +{ > + const char *regbuf; > + size_t size; > + > + gdb_assert (buf != NULL); > + assert_regnum (regnum); Should we assert that offset is < or <= size here too? Simon