This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH] gdb/microblaze-tdep.c: Check whether less than zero in conditional expression
- From: Michael Eager <eager at eagerm dot com>
- To: Chen Gang <gang dot chen dot 5i5j at gmail dot com>, Michael Eager <eager at eagercon dot com>
- Cc: binutils at sourceware dot org, gdb-patches at sourceware dot org
- Date: Wed, 23 Jul 2014 19:24:31 -0700
- Subject: Re: [PATCH] gdb/microblaze-tdep.c: Check whether less than zero in conditional expression
- Authentication-results: sourceware.org; auth=none
- References: <53CBCC2F dot 7040702 at gmail dot com> <53D01542 dot 9020107 at eagerm dot com> <53D031E7 dot 40602 at gmail dot com> <53D03483 dot 2060203 at eagercon dot com> <53D0352D dot 1020205 at gmail dot com> <53D0682E dot 9090201 at gmail dot com>
On 07/23/14 18:58, Chen Gang wrote:
On 07/24/2014 06:20 AM, Chen Gang wrote:
On 07/24/2014 06:17 AM, Michael Eager wrote:
On 07/23/14 15:06, Chen Gang wrote:
On 07/24/2014 04:04 AM, Michael Eager wrote:
On 07/20/14 07:03, Chen Gang wrote:
Use typecast 'size_t' on 'reg', not only avoid the related warning, but
also check whether less than zero -- for 'reg' is type 'int', and sizeof
(dwarf2_to_reg_map) is less than 0x7fff.
It is quoted in gdb_assert(), so need check 'reg' whether less than zero.
And the related warning (with '-W'):
../../binutils-gdb/gdb/microblaze-tdep.c:667:3: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
* microblaze-tdep.c (microblaze_dwarf2_reg_to_regnum): Check whether
less tha zero in conditional expression.
Signed-off-by: Chen Gang <firstname.lastname@example.org>
gdb/microblaze-tdep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/microblaze-tdep.c b/gdb/microblaze-tdep.c
index 7e89241..9bec260 100644
@@ -664,7 +664,7 @@ static int dwarf2_to_reg_map =
microblaze_dwarf2_reg_to_regnum (struct gdbarch *gdbarch, int reg)
- gdb_assert (reg < sizeof (dwarf2_to_reg_map));
+ gdb_assert ((size_t) reg < sizeof (dwarf2_to_reg_map));
I don't see anything in the patch which does what you describe,
checking whether reg is less than zero. Converting a signed
integer to an unsigned integer is not a way to check whether
it is less than zero. This is better:
+ gdb_assert (reg >= 0 && (size_t) reg < sizeof (dwarf2_to_reg_map));
Yeah, it is common statement. It is also OK to me, although after type
cast, 'reg >=0' can be omited (it can let code simpler, but let code
not quit easy understanding).
No, if you want to verify that the value is greater than zero,
this cannot be omitted. A negative value would converted to
a positive value by the cast. There no reason to believe that
this would cause the other half of the test to fail.
When an 'int' negative value converted to a positive value, it will be
larger than 0x7fff which must be larget than 'sizeof (dwarf2_to_reg_map)'.
If what I said is correct, your idea/suggestions is still OK to me: easy
understanding has higher priority than keeping source code simple.
Yes, you are correct. Took me a moment to think through.
I left your patch as is.
Michael Eager email@example.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077