[PATCH v2 02/14] lmb: Rename interior piece to area

Simon Glass sjg at chromium.org
Mon Sep 4 16:46:34 CEST 2023


Hi Heinrich,

On Fri, 1 Sept 2023 at 08:29, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Heinrich,
>
> On Fri, 1 Sept 2023 at 00:16, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >
> > On 9/1/23 03:13, Simon Glass wrote:
> > > The lmb data structures use the word 'region' to describe the region in
> > > which the allocations happen, as well as the individual allocations in
> > > that region. The interior structure is called lmb_property but is
> > > described as a region.
> > >
> > > This is quite confusing. One of the reviewers in v1 asked why we are using
> > > a property to describe a region!
> >
> > We currently have:
> >
> > struct lmb_region - Description of a set of region.
> > struct lmb_property - Description of one region.
> >
> > The word 'region' implies that it is contiguous, while 'region set'
> > implies that its members might not be contiguous.
>
> From what I can tell, the areas inside a region are contiguous. The
> code is so badly commented that it is hard to know what the intent
> was.
>
> Actually I just looked it up and found it came from Linux, so I
> suppose that explains the lack of comments. There it has been renamed
> memblock.
>
> Could you take a look and see if we could adopt the same naming?

I am waiting for your thoughts on this one.

>
> >
> > Calling a set region is what starts the confusion.
> >
> > Your patch is creating new confusion by calling a member of "a set of
> > regions" "area".
> >
> > Please, rename as follows:
> >
> > lmb_region -> lmb_region_set
>
> That sounds like it sets a region
>
> > lmb_property -> lmb_region
> >
> > The comments below are only valid if you stick with area which I
> > strongly discourage.
>
> Fair enough, I don't like it much either.
>
[..]

Regards,
Simon


More information about the U-Boot mailing list