This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Introduce in_inclusive_range, fix -Wtautological-compare warnings


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]