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

[Simon Marchi <simon.marchi@ericsson.com>]

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