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

Simon Marchi
2018-03-27T20:22:38Z

On 2018-03-27 12:53 AM, Tom Tromey wrote:
> Tom> I've run the new patch through the buildbot. I'll resubmit it soon, I
> Tom> just have to update the ChangeLog and rewrite the commit message.
>
> Here it is. Let me reiterate that this probably requires careful review
> as the buildbot may not test some of these paths.
>
> Tom

Hi Tom,

I had a first quick look, enough to familiarize myself with the problem.
Conceptually, the approach looks ok to me, but I'd like to have a closer
look later (unless someone else reviews it in the mean time).

Here are two small comments I noted.

> @@ -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;

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

> diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
> index 8c707aa8fe..880d87436e 100644
> --- a/gdb/xcoffread.c
> +++ b/gdb/xcoffread.c
> @@ -420,7 +420,11 @@ compare_lte (const void *lte1p, const void *lte2p)
> struct linetable_entry *lte1 = (struct linetable_entry *) lte1p;
> struct linetable_entry *lte2 = (struct linetable_entry *) lte2p;
>
> - return lte1->pc - lte2->pc;
> + if (lte1->raw_address () < lte2->raw_address ())
> + return -1;
> + if (lte1->raw_address () > lte2->raw_address ())
> + return -1;
> + return 0;
> }

Both branches return -1 here.

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

if (left->foo != right->foo)
return left->foo - right->foo;

return left->bar - right->bar;

Simon