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

Tom Tromey tom@tromey.com
Wed Mar 28 04:53:00 GMT 2018


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



More information about the Gdb-patches mailing list