Message ID | 1509373931-16340-1-git-send-email-simon.marchi@ericsson.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 15982 invoked by alias); 30 Oct 2017 14:33:04 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 15936 invoked by uid 89); 30 Oct 2017 14:33:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.2 spammy=HIGH, 11722, H*M:16340, H*r:sk:EUR01-H X-HELO: sesbmg23.ericsson.net Received: from sesbmg23.ericsson.net (HELO sesbmg23.ericsson.net) (193.180.251.37) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Oct 2017 14:32:59 +0000 Received: from ESESSHC022.ericsson.se (Unknown_Domain [153.88.183.84]) by sesbmg23.ericsson.net (Symantec Mail Security) with SMTP id 0B.6C.03220.91837F95; Mon, 30 Oct 2017 15:32:57 +0100 (CET) Received: from EUR01-HE1-obe.outbound.protection.outlook.com (153.88.183.145) by oa.msg.ericsson.com (153.88.183.84) with Microsoft SMTP Server (TLS) id 14.3.352.0; Mon, 30 Oct 2017 15:32:25 +0100 Received: from elxacz23q12.ca.am.ericsson.se (192.75.88.130) by AM3PR07MB305.eurprd07.prod.outlook.com (2a01:111:e400:881b::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.197.4; Mon, 30 Oct 2017 14:32:24 +0000 From: Simon Marchi <simon.marchi@ericsson.com> To: <gdb-patches@sourceware.org> CC: Simon Marchi <simon.marchi@ericsson.com> Subject: [PATCH] Introduce in_inclusive_range, fix -Wtautological-compare warnings Date: Mon, 30 Oct 2017 10:32:11 -0400 Message-ID: <1509373931-16340-1-git-send-email-simon.marchi@ericsson.com> In-Reply-To: <189cefb7-aa0c-16bd-d78c-cc6f1e5c1344@baldwin.cx> References: <189cefb7-aa0c-16bd-d78c-cc6f1e5c1344@baldwin.cx> MIME-Version: 1.0 Content-Type: text/plain X-ClientProxiedBy: CY4PR13CA0076.namprd13.prod.outlook.com (2603:10b6:903:152::14) To AM3PR07MB305.eurprd07.prod.outlook.com (2a01:111:e400:881b::13) X-MS-Office365-Filtering-Correlation-Id: 3161a9eb-7d50-412d-d603-08d51fa30a0d X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(4534020)(4602075)(2017052603238); SRVR:AM3PR07MB305; X-Microsoft-Exchange-Diagnostics: 1; AM3PR07MB305; 3:VZZfi9xTWhFJ+qfFhKpWbZTJv/ktvNohezN8tekbu2c0OkTCjIOooRCaN9FLiBS410P1bC1ZdNIxr7NE8SRdv7CCYoC6qJ9jBbwxNQpsDMr/SiCycDKYBkqkax7IE6+A9kW/MfnozK6btL3cykQlzmRyYGutuHXTsZ7ZT9Ax5WkZaccW62oKXX7oJgujh3xejMSGhJXp21a8srFGAc+FwN6qpP0Odr4DOo3FUk+WAprvzp6TmKLeY94M31p4QamS; 25:sQrxKc6l3aq9R/dsmdVLAWcYDOCx0zwEqLbH4Jd3fXgJOHcoAbDA3HHeqawTh1OqWuUlj/zFcgxYKUC9a4tn/eKX+FNp/5mfwyj4qPMRURu/zqF06gaH92w5rVJbtr3jNIMKEVeHbxxiSRviZ/b4kc1XgTazpD9P14F4ElMSAvEqg0+SoEN4OiTdv4uCgaBLfZeQ8s9fU3S477v/MRorXdxfSQJyfaILenWBiDIt/GTOmYDeyrlO3x0VR2RAVIl6bxOjmNpronD+PV22lAAsUM8XyHVQkcsQa7qbMqlZF+zZNF6lJUjbdVDitfduNB451UU2lw8leUbeEKxgtEB8IwkzD8kNCO5M9ydkhFdcVSo=; 31:l/h6hJmk6k00dF1cWI99Rvn1Dq23EKyoohp6dvXI6Cjy+9kVDOLJGWG7n0+A2CJ8z1wGDG9bSvE9BpRQEHzowK/lXVYp+GpPwx4dOmS6hW8Wc9HkLakxI9WZ9dH9UfC9JtdE1rDogWUCAEOjBVgihJjzFPXiFQ6nJEZPoY5LHRK20meBzJKlRmy74oasoSIGJwHPGMpyZYZuwzhgiF5dktUTeI1nan6GxHMh/Eg6B9Y= X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AM3PR07MB305: Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; X-Microsoft-Exchange-Diagnostics: 1; AM3PR07MB305; 20:aDPuPDg9yq2zSumRIhaVRQoSiyYe9u7646msUtFvsbTxrMZD6eahaVGf/0GEh4jVgc+8t79NccgplM05ATvGwwHcmfgPyJrduLMoZ3b5nJCRUMthhagMALuZgdJVw50IajBGzMKwsuTkiO1aJcIhW+LYTGTwLqf7BmoeRnp1ffyQYsNddVj0TSW7KPZchDepvP+usMNZU9aU+r30QnXZvQvbPhrR4Yi+g10r9ls066r1lYlGtdcPpHoKAk+fvqpT2FD+cy/g7iFCtqUD2xdWQoiq0xFfV+5lEOs+4jTG1tHsvO9w+H5jJMcCnTZvvKstQYEK+tqShxAlIVU12iOnnWIvvT8tx6Iy8wifr3ExLyur1S90xYsUZ15c4Zcrta19olInylV3mVJKcuY9Wc3cFAykLzQezcUAo3+Myq8bLv5vFR6l+z/9KgubJPb5HpWD/+FCHIUa6PMhz/VUAPX1UYGU8Qx4i/Pun7KIxpkgyU4oAqawXlJE14SzvnzFkFk0; 4:jgizsnojuEABJ0oH6dD5YgSiRzVkTwJboDgnK0JEtMqLTtT7gI6+D6VJw9+0kieVaUhJuKd0AQHsTQDfPb8BReB6BJBjrb4tUi8hCvNiEM0Fwv0JvmSzz4lLFP+UTqZWSBU2FeVe1L44GLWnx1XHfOlVijFhZf0CsOInaY+TetKnwB4SBcKJmXe1mFMPs+Dx/yjTHxx0qxPssdEHcQEmAlctZDiSqfBj1tXryiDlhGZeZYUHFSRR8df04qwMvccA5iZo+tuFVxMzFNegu5AYWw== X-Exchange-Antispam-Report-Test: UriScan:; X-Microsoft-Antispam-PRVS: <AM3PR07MB3058961E669A2D24A0CD630ED590@AM3PR07MB305.eurprd07.prod.outlook.com> X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(3231020)(93006095)(93001095)(3002001)(100000703101)(100105400095)(10201501046)(6041248)(20161123562025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123558100)(20161123555025)(20161123564025)(20161123560025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:AM3PR07MB305; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:AM3PR07MB305; X-Forefront-PRVS: 0476D4AB88 X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(6009001)(376002)(346002)(39860400002)(54534003)(199003)(189002)(5003940100001)(97736004)(2906002)(86362001)(5660300001)(107886003)(6506006)(6512007)(16586007)(316002)(81156014)(53936002)(6486002)(189998001)(8676002)(81166006)(68736007)(478600001)(50466002)(48376002)(2361001)(2351001)(106356001)(33646002)(50226002)(4326008)(66066001)(36756003)(25786009)(6116002)(3846002)(305945005)(16526018)(6916009)(6666003)(47776003)(50986999)(2950100002)(76176999)(7736002)(8936002)(105586002)(101416001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM3PR07MB305; H:elxacz23q12.ca.am.ericsson.se; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; Received-SPF: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; AM3PR07MB305; 23:mNfwD6yC4bgRgJO0ft948uP+LIb/iIPYC/Q0ebPQGD?= =?us-ascii?Q?d4ewsH5D2BFa5D2WE42Z3FQQRA8fCAiZDsPRQLgFS56XoO8I8kWGP3//fhO9?= =?us-ascii?Q?4j95zsK1YRuF1Y8pleDAZTk0x5K/AJ6BrGen46dYkxitSnNSK4IAEMJHKF5J?= =?us-ascii?Q?ob9ArDyAIFX+8DaMElpcBfSqOfTRC/1N5TjlekElgdKJ+6KpcVeeldIlA4ly?= =?us-ascii?Q?vGweaExREqgiqP9/dIQ0OPULXKQy1FdDvmtOFPCHXt43q22sWiu5/poelX1N?= =?us-ascii?Q?2vq+TMSHAYbtkWfvUb+zkjlhQb2g55Erkn/64r7xsByKJHv6T8B30firm2V4?= =?us-ascii?Q?tp6HwX5WxtEEo7HA7IHK1VjWq9uPtG81G3RXyN67OLid06mpT0XNgjikqNHr?= =?us-ascii?Q?AJASt/MOQZSnwaPJTTCzWp6gu7z4eyxdRSoWdg2VqCV6UdC0bK9Bza5Q8/EK?= =?us-ascii?Q?/l0oVaR3C3nxQ6NR3Xs41t8qo8eA9tsRVD2Wt5Knyxj5Z2D9JdPq7lrZK19d?= =?us-ascii?Q?kDtnwTOg76MnVDyRbnZM2UP7HnJtbMrKHBDjFHcxvAtV0BWG6875rPEJYBDQ?= =?us-ascii?Q?HQzTj2xSZWWroNQRIjTPdRMfJMlHffERQXBFo1LBvnmbF5xEcbxp3bdf0rBK?= =?us-ascii?Q?OHaYmE6kmSdH0ehv/ZYBYmwT7QGHoXUfM757NyYma8TpDPBIQHgU9eFmr80/?= =?us-ascii?Q?+Qb5H0DOPALUa4ekqkv581Gynkdl9YIg1DHk+20JdotucWWmtOCQOPXas0D/?= =?us-ascii?Q?FXrS0BJS0UQ7wNKgeYVR8S1fBaPXzz5X0fo+7sL0z6mPpKL01sCtZXcOH092?= =?us-ascii?Q?+bIN98dRZ1AreO2jned13wSfIJ5hpI0tnhko0jnHW60vPwv7alV3q+0BxvVH?= =?us-ascii?Q?BDuNWp8luyeO+Px3B+PW16/QboaG5NGB1Cl7vSfIsVQrzT6hRtJn2HjR8xIx?= =?us-ascii?Q?qntz0ouNFjo4dzk8LbfYtJiHQp7qAQmn/DMXKMQDIB4h6OrN79VlVZT9FFmB?= =?us-ascii?Q?WDtQJg/KrbN5maw5YBy0demE/Lqrvb0V6hCjP4PWzFL6sN9hPKSn0hPL8NRG?= =?us-ascii?Q?faJiGtG8MXdbPNQyVzfWewFG4nASYpTXSmGt0BUlQTXpRxmz2UOknSXDJbXG?= =?us-ascii?Q?leDH6FtMH47Eo5o+vItSaT935j1czTQ/XXtTnEsKmQZI6yMgaUpw=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1; AM3PR07MB305; 6:nGo4XWlPVvWCI/fWCQglxGHEuXpP56OXfD87wvGCdLcMV0cM/bLaG1nFT+K5AMG881q+r004YTeVUef8PWR7xMvonecDRwE/k6ynN36Zib7eLX5TtIKVGh01NSJKNBJwkFjUPe7/Od6H4xMo2XnoAXIR2DdRSQEnHbTeUmqhLWEc7rEhuwI4yjIdUsVCKSlRSe4OB89UcQaCfhO1fvcc3ZI2fz92jNuyO4fqPxRvu/rI60Gee2o5WZJGALCM+BBdLQjWR8t8c4FznIDrUBaWtY0cNd/gT+eklTQHRDHa0MX0PbBc4sCEkC1KUfMPMJYg13awEM1yvy76T3KHwp8mRv+xr736HIEocJ2eSx1N4a4=; 5:oua4xQdtI3r95jKoa91bAp3XD4PSlBEihhetwQ8MjQwkB6NhRJXdhVuerckQdaOAMHQanHTuhVCuodN1F03Y03WxglF9uKp1SaB3iZFjQEFCHxmi9t8u9SCNFuuNyG4q81WkLRG9FgUuIFFwL8G6IDc7hpfeUwFjb/Z04Eu9MvA=; 24:5jRCuBstLfAjSWVZ2iqUDjj/xPwSi5iv7ubUDaDgYX7YBW6BSixLAwGWDVfXa4C74Ndk1i+t51bZlqyxfnhy9i00xYYfCsEL2kkSOGl38SE=; 7:8gjh/UB2XY9o/eRgakvOa1KL1ZFD4yjxB5aIyNi35VpIjSug/GMk9qRsAimZwaOTS1uSXPaHnjoZkBI0GSdRqwrKiMyp5ZDBpJC9hL2vyd7bVmHasW2RrnAdqSFb1Bx8iLhXTwmVLZ0aiox0++BHVfNAZRzuyLvQ7RsgDJbHvcOnZMGNMUfdeTj8349tmenk76vZJxOWEfZeHFn5rXQOwewMaM0TkJHt4ZVC23QvNlmaKk0mdtRFxVxEr37Iy1U2 SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Oct 2017 14:32:24.4102 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 3161a9eb-7d50-412d-d603-08d51fa30a0d X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 92e84ceb-fbfd-47ab-be52-080c6b87953f X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM3PR07MB305 X-OriginatorOrg: ericsson.com X-IsSubscribed: yes |
Commit Message
Simon Marchi
Oct. 30, 2017, 2:32 p.m. UTC
When compiling with clang or gcc 8, we see warnings like this: /home/emaisin/src/binutils-gdb/gdb/arm-tdep.c:10013:13: error: comparison of 0 <= unsigned expression is always true [-Werror,-Wtautological-compare] if (0 <= insn_op1 && 3 >= insn_op1) ~ ^ ~~~~~~~~ /home/emaisin/src/binutils-gdb/gdb/arm-tdep.c:11722:20: error: comparison of unsigned expression >= 0 is always true [-Werror,-Wtautological-compare] else if (opB >= 0 && opB <= 2) ~~~ ^ ~ This is because an unsigned integer (opB in this case) will always be >= 0. It is still useful to keep both bounds of the range in the expression, even if one is at the edge of the data type range. This patch introduces a utility function in_inclusive_range that gets rid of the warning while conveying that we are checking for a range. Tested by rebuilding. gdb/ChangeLog: * common/common-utils.h (in_inclusive_range): New function. * arm-tdep.c (arm_record_extension_space): Use in_inclusive_range. * cris-tdep.c (cris_spec_reg_applicable): Use in_inclusive_range. --- gdb/arm-tdep.c | 4 ++-- gdb/common/common-utils.h | 9 +++++++++ gdb/cris-tdep.c | 4 ++-- 3 files changed, 13 insertions(+), 4 deletions(-)
Comments
On 10/30/17 2:32 PM, Simon Marchi wrote: > When compiling with clang or gcc 8, we see warnings like this: > > /home/emaisin/src/binutils-gdb/gdb/arm-tdep.c:10013:13: error: comparison of 0 <= unsigned expression is always true [-Werror,-Wtautological-compare] > if (0 <= insn_op1 && 3 >= insn_op1) > ~ ^ ~~~~~~~~ > /home/emaisin/src/binutils-gdb/gdb/arm-tdep.c:11722:20: error: comparison of unsigned expression >= 0 is always true [-Werror,-Wtautological-compare] > else if (opB >= 0 && opB <= 2) > ~~~ ^ ~ > > This is because an unsigned integer (opB in this case) will always be >= > 0. It is still useful to keep both bounds of the range in the > expression, even if one is at the edge of the data type range. This > patch introduces a utility function in_inclusive_range that gets rid of > the warning while conveying that we are checking for a range. > > Tested by rebuilding. > > gdb/ChangeLog: > > * common/common-utils.h (in_inclusive_range): New function. > * arm-tdep.c (arm_record_extension_space): Use > in_inclusive_range. > * cris-tdep.c (cris_spec_reg_applicable): Use > in_inclusive_range. > --- > gdb/arm-tdep.c | 4 ++-- > gdb/common/common-utils.h | 9 +++++++++ > gdb/cris-tdep.c | 4 ++-- > 3 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c > index d623eb6..209e29f 100644 > --- a/gdb/cris-tdep.c > +++ b/gdb/cris-tdep.c > @@ -1434,7 +1434,7 @@ cris_spec_reg_applicable (struct gdbarch *gdbarch, > /* Indeterminate/obsolete. */ > return 0; > case cris_ver_v0_3: > - return (version >= 0 && version <= 3); > + return in_inclusive_range (version, 0U, 3U); > case cris_ver_v3p: > return (version >= 3); > case cris_ver_v8: > @@ -1442,7 +1442,7 @@ cris_spec_reg_applicable (struct gdbarch *gdbarch, > case cris_ver_v8p: > return (version >= 8); > case cris_ver_v0_10: > - return (version >= 0 && version <= 10); > + return in_inclusive_range (version, 0U, 10U); > case cris_ver_v3_10: > return (version >= 3 && version <= 10); > case cris_ver_v8_10: I wonder if in this file it wouldn't be best to use the new function throughout the various cases so that the style is more consistent? LGTM regardless.
On 2017-10-30 11:57, John Baldwin wrote: > On 10/30/17 2:32 PM, Simon Marchi wrote: >> When compiling with clang or gcc 8, we see warnings like this: >> >> /home/emaisin/src/binutils-gdb/gdb/arm-tdep.c:10013:13: error: >> comparison of 0 <= unsigned expression is always true >> [-Werror,-Wtautological-compare] >> if (0 <= insn_op1 && 3 >= insn_op1) >> ~ ^ ~~~~~~~~ >> /home/emaisin/src/binutils-gdb/gdb/arm-tdep.c:11722:20: error: >> comparison of unsigned expression >= 0 is always true >> [-Werror,-Wtautological-compare] >> else if (opB >= 0 && opB <= 2) >> ~~~ ^ ~ >> >> This is because an unsigned integer (opB in this case) will always be >> >= >> 0. It is still useful to keep both bounds of the range in the >> expression, even if one is at the edge of the data type range. This >> patch introduces a utility function in_inclusive_range that gets rid >> of >> the warning while conveying that we are checking for a range. >> >> Tested by rebuilding. >> >> gdb/ChangeLog: >> >> * common/common-utils.h (in_inclusive_range): New function. >> * arm-tdep.c (arm_record_extension_space): Use >> in_inclusive_range. >> * cris-tdep.c (cris_spec_reg_applicable): Use >> in_inclusive_range. >> --- >> gdb/arm-tdep.c | 4 ++-- >> gdb/common/common-utils.h | 9 +++++++++ >> gdb/cris-tdep.c | 4 ++-- >> 3 files changed, 13 insertions(+), 4 deletions(-) >> >> diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c >> index d623eb6..209e29f 100644 >> --- a/gdb/cris-tdep.c >> +++ b/gdb/cris-tdep.c >> @@ -1434,7 +1434,7 @@ cris_spec_reg_applicable (struct gdbarch >> *gdbarch, >> /* Indeterminate/obsolete. */ >> return 0; >> case cris_ver_v0_3: >> - return (version >= 0 && version <= 3); >> + return in_inclusive_range (version, 0U, 3U); >> case cris_ver_v3p: >> return (version >= 3); >> case cris_ver_v8: >> @@ -1442,7 +1442,7 @@ cris_spec_reg_applicable (struct gdbarch >> *gdbarch, >> case cris_ver_v8p: >> return (version >= 8); >> case cris_ver_v0_10: >> - return (version >= 0 && version <= 10); >> + return in_inclusive_range (version, 0U, 10U); >> case cris_ver_v3_10: >> return (version >= 3 && version <= 10); >> case cris_ver_v8_10: > > I wonder if in this file it wouldn't be best to use the new function > throughout > the various cases so that the style is more consistent? LGTM > regardless. Good point, I'll make that change. I'll see if it's possible in arm-tdep.c too or if it's a too daunting task. Simon
On 10/30/17 4:01 PM, Simon Marchi wrote: > On 2017-10-30 11:57, John Baldwin wrote: >> On 10/30/17 2:32 PM, Simon Marchi wrote: >>> When compiling with clang or gcc 8, we see warnings like this: >>> >>> /home/emaisin/src/binutils-gdb/gdb/arm-tdep.c:10013:13: error: >>> comparison of 0 <= unsigned expression is always true >>> [-Werror,-Wtautological-compare] >>> if (0 <= insn_op1 && 3 >= insn_op1) >>> ~ ^ ~~~~~~~~ >>> /home/emaisin/src/binutils-gdb/gdb/arm-tdep.c:11722:20: error: >>> comparison of unsigned expression >= 0 is always true >>> [-Werror,-Wtautological-compare] >>> else if (opB >= 0 && opB <= 2) >>> ~~~ ^ ~ >>> >>> This is because an unsigned integer (opB in this case) will always be >>>> = >>> 0. It is still useful to keep both bounds of the range in the >>> expression, even if one is at the edge of the data type range. This >>> patch introduces a utility function in_inclusive_range that gets rid >>> of >>> the warning while conveying that we are checking for a range. >>> >>> Tested by rebuilding. >>> >>> gdb/ChangeLog: >>> >>> * common/common-utils.h (in_inclusive_range): New function. >>> * arm-tdep.c (arm_record_extension_space): Use >>> in_inclusive_range. >>> * cris-tdep.c (cris_spec_reg_applicable): Use >>> in_inclusive_range. >>> --- >>> gdb/arm-tdep.c | 4 ++-- >>> gdb/common/common-utils.h | 9 +++++++++ >>> gdb/cris-tdep.c | 4 ++-- >>> 3 files changed, 13 insertions(+), 4 deletions(-) >>> >>> diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c >>> index d623eb6..209e29f 100644 >>> --- a/gdb/cris-tdep.c >>> +++ b/gdb/cris-tdep.c >>> @@ -1434,7 +1434,7 @@ cris_spec_reg_applicable (struct gdbarch >>> *gdbarch, >>> /* Indeterminate/obsolete. */ >>> return 0; >>> case cris_ver_v0_3: >>> - return (version >= 0 && version <= 3); >>> + return in_inclusive_range (version, 0U, 3U); >>> case cris_ver_v3p: >>> return (version >= 3); >>> case cris_ver_v8: >>> @@ -1442,7 +1442,7 @@ cris_spec_reg_applicable (struct gdbarch >>> *gdbarch, >>> case cris_ver_v8p: >>> return (version >= 8); >>> case cris_ver_v0_10: >>> - return (version >= 0 && version <= 10); >>> + return in_inclusive_range (version, 0U, 10U); >>> case cris_ver_v3_10: >>> return (version >= 3 && version <= 10); >>> case cris_ver_v8_10: >> >> I wonder if in this file it wouldn't be best to use the new function >> throughout >> the various cases so that the style is more consistent? LGTM >> regardless. > > > Good point, I'll make that change. I'll see if it's possible in > arm-tdep.c too or if it's a too daunting task. It may be sufficient to just be consistent within a function? In arm-tdep.c it is two instances in separate functions whereas for cris-tdep.c it is multiple instances in the same function which is what stuck out to me.
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index def3958..282efe0 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -10010,7 +10010,7 @@ arm_record_extension_space (insn_decode_record *arm_insn_r) && !INSN_RECORDED(arm_insn_r)) { /* Handle MLA(S) and MUL(S). */ - if (0 <= insn_op1 && 3 >= insn_op1) + if (in_inclusive_range (insn_op1, 0U, 3U)) { record_buf[0] = bits (arm_insn_r->arm_insn, 12, 15); record_buf[1] = ARM_PS_REGNUM; @@ -11719,7 +11719,7 @@ thumb_record_ld_st_reg_offset (insn_decode_record *thumb_insn_r) record_buf[0] = reg_src1; thumb_insn_r->reg_rec_count = 1; } - else if (opB >= 0 && opB <= 2) + else if (in_inclusive_range (opB, 0U, 2U)) { /* STR(2), STRB(2), STRH(2) . */ reg_src1 = bits (thumb_insn_r->arm_insn, 3, 5); diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h index a32863c..4926a32 100644 --- a/gdb/common/common-utils.h +++ b/gdb/common/common-utils.h @@ -125,4 +125,13 @@ extern void free_vector_argv (std::vector<char *> &v); joining all the arguments with a whitespace separating them. */ extern std::string stringify_argv (const std::vector<char *> &argv); +/* Return true if VALUE is in [LOW, HIGH]. */ + +template <typename T> +static bool +in_inclusive_range (T value, T low, T high) +{ + return value >= low && value <= high; +} + #endif diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c index d623eb6..209e29f 100644 --- a/gdb/cris-tdep.c +++ b/gdb/cris-tdep.c @@ -1434,7 +1434,7 @@ cris_spec_reg_applicable (struct gdbarch *gdbarch, /* Indeterminate/obsolete. */ return 0; case cris_ver_v0_3: - return (version >= 0 && version <= 3); + return in_inclusive_range (version, 0U, 3U); case cris_ver_v3p: return (version >= 3); case cris_ver_v8: @@ -1442,7 +1442,7 @@ cris_spec_reg_applicable (struct gdbarch *gdbarch, case cris_ver_v8p: return (version >= 8); case cris_ver_v0_10: - return (version >= 0 && version <= 10); + return in_inclusive_range (version, 0U, 10U); case cris_ver_v3_10: return (version >= 3 && version <= 10); case cris_ver_v8_10: