Re: [PATCH] Fix thinko on common/offset-type.h (compare 'lhs' against 'rhs')

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Friday, October 26 2018, Kevin Buettner wrote:

> On Thu, 25 Oct 2018 17:10:08 -0400
> Sergio Durigan Junior <sergiodj@redhat.com> wrote:
>
>> While doing something else, I noticed that the OFFSET_TYPE's
>> "DEFINE_OFFSET_REL_OP" has a thinko: it is comparing 'lhs' against
>> itself, instead of against 'rhs'.  This patch fixes it.
>> 
>> I also found an interesting thing.  We have an unittest for
>> offset-type, and in theory it should have caught this problem, because
>> it has tests for relational operators.  However, the tests
>> successfully pass, and after some investigation I'm almost sure this
>> is because these operators are not being properly overloaded.  I tried
>> a few things to make them be used, without success.  If someone wants
>> to give this a try, I'd appreciate.
>> 
>> No regressions introduced.
>> 
>> gdb/ChangeLog:
>> 2018-10-25  Sergio Durigan Junior  <sergiodj@redhat.com>
>> 
>> 	* common/offset-type.h (DEFINE_OFFSET_REL_OP): Compare 'lhs'
>> 	against 'rhs', instead of with 'lhs' again.
>
> LGTM.
>
> (I'm surprised that it wasn't caught by the unit test or by someone
> else noticing a bug elsewhere in GDB.)

Thanks for the review, Kevin.

According to my discussion with Simon, the proposed approach (for which
I'll submit a new patch soon) is to entirely remove the overloads for
relational operators, since they're clearly not being used.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/