This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH][PING][PR gdb/19361] Fix invalid comparison functions
- From: Yuri Gribov <tetra2005 at gmail dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Yury Gribov <y dot gribov at samsung dot com>, gdb-patches at sourceware dot org, Stan Shebs <stanshebs at google dot com>, Paul Pluzhnikov <ppluzhnikov at google dot com>
- Date: Sat, 2 Jan 2016 05:18:32 +0300
- Subject: Re: [PATCH][PING][PR gdb/19361] Fix invalid comparison functions
- Authentication-results: sourceware.org; auth=none
- References: <566FFEE2 dot 4010300 at samsung dot com> <56823702 dot 6020804 at samsung dot com> <5682C264 dot 3030109 at redhat dot com> <5682CC66 dot 70608 at samsung dot com> <CAJOtW+7zb01iC7y4unfo6vVPN2z4_eHoD7xn93f-9tRfy7ffag at mail dot gmail dot com> <56844BE3 dot 9030508 at redhat dot com> <56844E1B dot 20507 at redhat dot com>
On Thu, Dec 31, 2015 at 12:35 AM, Pedro Alves <palves@redhat.com> wrote:
> On 12/30/2015 09:25 PM, Pedro Alves wrote:
>> On 12/30/2015 08:18 PM, Yuri Gribov wrote:
>>
>>> Sorry, I should have been more wordy about the actual problem. With
>>> current approach i.e.
>>>
>>> if (pid1 == pgid1)
>>> return -1;
>>> else if (pid2 == pgid2)
>>> return 1;
>>>
>>> comparison of two group leaders is not going to be symmetric:
>>>
>>> cmp(lead_1, lead_2) == cmp(lead_2, lead_1) == -1
>>
>> Aaaaaaah, d'oh! Thanks, it's obvious now, yes, we fail to consider
>> the case of both elements being leaders. I couldn't see that
>> even after staring at the code for a while. That hunk is OK as
>> is then. (Please clarify this in the commit log.)
>
> Wait, no we don't... If both are leaders when you get there, then
> they must have different pgid's, and that case is handled before:
>
> /* Sort by PGID. */
> if (pgid1 < pgid2)
> return -1;
> else if (pgid1 > pgid2)
> return 1;
> else
>
> But let's assume not. Let's assume we see two leaders when you get to the
> code in question. That means they have pgid1==pgid2. Since by definition
> being leader means pgid==pid, it must be that pid1 == pid2 as well. That
> is, this is really about comparing equivalent elements. Which
> brings us back again to:
>
> /* Easier to check for equivalent element first. */
> if (pid1 == pid2)
> return 0;
>
> Or am I confused again?
Hm, this sounds reasonable. Let me see if I can repro this bug and get
the exact inputs which caused the problem (unfortunately I didn't
collect them during testing).
-Y