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: Pedro Alves <palves at redhat dot com>
- To: Yuri Gribov <tetra2005 at gmail dot com>, Yury Gribov <y dot gribov at samsung dot com>
- Cc: gdb-patches at sourceware dot org, Stan Shebs <stanshebs at google dot com>, Paul Pluzhnikov <ppluzhnikov at google dot com>
- Date: Wed, 30 Dec 2015 21:35:23 +0000
- 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>
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?
Thanks,
Pedro Alves