Re: [PATCH v4 1/2] semihosting/arm-compat: replace heuristic for softmmu SYS_HEAPINFO

[Peter Maydell <peter.maydell@linaro.org>]

On Wed, 9 Feb 2022 at 16:31, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Wed, 23 Jun 2021 at 14:47, Alex Bennée <alex.bennee@linaro.org> wrote:
> >> +static gint sort_secs(gconstpointer a, gconstpointer b)
> >> +{
> >> +    RomSec *ra = (RomSec *) a;
> >> +    RomSec *rb = (RomSec *) b;
> >
> > /*
> >  * Sort into address order. We break ties between rom-startpoints
> >  * and rom-endpoints in favour of the startpoint, by sorting the 0->1
> >  * transition before the 1->0 transition. Either way round would
> >  * work, but this way saves a little work later by avoiding
> >  * dealing with "gaps" of 0 length.
> >  */
> >
> >> +
> >> +    if (ra->base == rb->base) {
> >> +        return ra->se > rb->se ? -1 : 1;
> >> +    }
> >> +    return ra->base > rb->base ? 1 : -1;
> >
> > This has forgotten the "equality" case, which you will
> > see if two blobs start at the same address (at least in
> > theory; at the moment the rom blob loader will try to
> > reject overlaps, though it might not do so forever).
>
> I'm confused what you mean by equality case. If both RomSecs have the
> same base and the same se flag we need to pick one of them. Are you
> saying when ra->se == ra->sb we should take ra?

I'm saying that a sort comparison should give a consistent answer.
At the moment if you pass it two items A and B which happen to
have the same base and se values, then if you call sort_secs(A, B)
it will claim "B is greater than A", but if you call sort_secs(B, A)
it will claim "A is greater than B". The GCompareFunc API provides
a way to say "these are the same" -- return 0.

-- PMM