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

[Tom Tromey <tom at tromey dot com>]

>>>>> "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