Re: [RFA 1/2] Make line tables independent of progspace

Tom Tromey
2018-03-28T04:53:44Z

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> Here are two small comments I noted.

Thank you.

>> @@ -517,10 +522,12 @@ do_mixed_source_and_assembly (struct gdbarch *gdbarch,
>> first_le = NULL;
>>
>> /* Skip all the preceding functions. */
>> - for (i = 0; i < nlines && le[i].pc < low; i++)
>> - continue;
>> + for (i = 0;
>> + i < nlines - 1 && le[i].address (main_symtab) < low;

Simon> I don't understand why "i < nlines" becomes "i < nlines - 1" here.

Bad change on my part.

>> + if (lte1->raw_address () < lte2->raw_address ())
>> + return -1;
>> + if (lte1->raw_address () > lte2->raw_address ())
>> + return -1;
>> + return 0;

Simon> Both branches return -1 here.

Ugh, sorry. This got introduced during the changes to use member
functions.

Simon> I don't mind the "left - right", I see that often. Especially when you
Simon> want to sort by multiple fields, it's short and clear (IMO) to do

In this particular case I think it is a bit weird, because the addresses
are unsigned, so this is relying on the implicit cast to make the value
negative. But also, because the addresses are CORE_ADDR and the return
type is int, it seems like underflow is possible as well (in theory, I
suppose I wouldn't expect it in practice).

So, I thought rather than being tricky here, it was more obviously
correct to just write the longer form. Which would be true if it didn't
introduce bugs.

Tom