| Message ID | f978a4677b36b0505994ae4d5b5c91efe090b6bd.1409018983.git.peter.crosthwaite@xilinx.com |
|---|---|
| State | New |
| Headers | show |
Il 26/08/2014 05:10, Peter Crosthwaite ha scritto: > To support name retrieval of MemoryRegions that were created > dynamically (that is, not via memory_region_init and friends). We > cache the name in MemoryRegion's state as > object_get_canonical_path_component mallocs the returned value > so it's not suitable for direct return to callers. Memory already > frees the name field, so this will be garbage collected along with > the MR object. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > > memory.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/memory.c b/memory.c > index 42317a2..fc16e5f 100644 > --- a/memory.c > +++ b/memory.c > @@ -914,7 +914,6 @@ void memory_region_init(MemoryRegion *mr, > if (size == UINT64_MAX) { > mr->size = int128_2_64(); > } > - mr->name = g_strdup(name); > > if (name) { > object_property_add_child_array(owner, name, OBJECT(mr)); > @@ -1309,6 +1308,10 @@ uint64_t memory_region_size(MemoryRegion *mr) > > const char *memory_region_name(const MemoryRegion *mr) > { > + if (!mr->name) { > + ((MemoryRegion *)mr)->name = > + object_get_canonical_path_component(OBJECT(mr)); > + } > return mr->name; > } > > Oh if we only used C++... :) Thanks, looks good! Paolo
Il 26/08/2014 05:10, Peter Crosthwaite ha scritto: > To support name retrieval of MemoryRegions that were created > dynamically (that is, not via memory_region_init and friends). We > cache the name in MemoryRegion's state as > object_get_canonical_path_component mallocs the returned value > so it's not suitable for direct return to callers. Memory already > frees the name field, so this will be garbage collected along with > the MR object. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > > memory.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/memory.c b/memory.c > index 42317a2..fc16e5f 100644 > --- a/memory.c > +++ b/memory.c > @@ -914,7 +914,6 @@ void memory_region_init(MemoryRegion *mr, > if (size == UINT64_MAX) { > mr->size = int128_2_64(); > } > - mr->name = g_strdup(name); This causes a bunch of [0] to appear in the "info mtree" output. They're a bit ugly, so I'm removing this line for now. Paolo > if (name) { > object_property_add_child_array(owner, name, OBJECT(mr)); > @@ -1309,6 +1308,10 @@ uint64_t memory_region_size(MemoryRegion *mr) > > const char *memory_region_name(const MemoryRegion *mr) > { > + if (!mr->name) { > + ((MemoryRegion *)mr)->name = > + object_get_canonical_path_component(OBJECT(mr)); > + } > return mr->name; > } > >
On Fri, Aug 29, 2014 at 12:06 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 26/08/2014 05:10, Peter Crosthwaite ha scritto: >> To support name retrieval of MemoryRegions that were created >> dynamically (that is, not via memory_region_init and friends). We >> cache the name in MemoryRegion's state as >> object_get_canonical_path_component mallocs the returned value >> so it's not suitable for direct return to callers. Memory already >> frees the name field, so this will be garbage collected along with >> the MR object. >> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> --- >> >> memory.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/memory.c b/memory.c >> index 42317a2..fc16e5f 100644 >> --- a/memory.c >> +++ b/memory.c >> @@ -914,7 +914,6 @@ void memory_region_init(MemoryRegion *mr, >> if (size == UINT64_MAX) { >> mr->size = int128_2_64(); >> } >> - mr->name = g_strdup(name); > > This causes a bunch of [0] to appear in the "info mtree" output. > They're a bit ugly, so I'm removing this line for now. > Yeh, so a solution to that was to patch qom to drop the [*] for singletons added via arrayified property adders. One possible semantic that I can think of is the 0th element always has a property alias so that foo and foo[0] mean the same thing. This tricky part is, when "[1]" comes along the preferred name (the canonical path) for the 0th element should switch from "foo" to "foo[0]". Is it sane to switch child and alias strings for "[0]" when "[1]" comes along? Regards, Peter > Paolo > >> if (name) { >> object_property_add_child_array(owner, name, OBJECT(mr)); >> @@ -1309,6 +1308,10 @@ uint64_t memory_region_size(MemoryRegion *mr) >> >> const char *memory_region_name(const MemoryRegion *mr) >> { >> + if (!mr->name) { >> + ((MemoryRegion *)mr)->name = >> + object_get_canonical_path_component(OBJECT(mr)); >> + } >> return mr->name; >> } >> >> > >
Il 28/08/2014 16:27, Peter Crosthwaite ha scritto: > Yeh, so a solution to that was to patch qom to drop the [*] for > singletons added via arrayified property adders. One possible semantic > that I can think of is the 0th element always has a property alias so > that foo and foo[0] mean the same thing. This tricky part is, when > "[1]" comes along the preferred name (the canonical path) for the 0th > element should switch from "foo" to "foo[0]". Is it sane to switch > child and alias strings for "[0]" when "[1]" comes along? You and I both know that the right solution is to move the [*] from memory.c to the callers... :) Paolo
On Fri, Aug 29, 2014 at 12:57 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 28/08/2014 16:27, Peter Crosthwaite ha scritto: >> Yeh, so a solution to that was to patch qom to drop the [*] for >> singletons added via arrayified property adders. One possible semantic >> that I can think of is the 0th element always has a property alias so >> that foo and foo[0] mean the same thing. This tricky part is, when >> "[1]" comes along the preferred name (the canonical path) for the 0th >> element should switch from "foo" to "foo[0]". Is it sane to switch >> child and alias strings for "[0]" when "[1]" comes along? > > You and I both know that the right solution is to move the [*] from > memory.c to the callers... :) > Hmm that's a lot of changes :( Regards, Peter > Paolo >
diff --git a/memory.c b/memory.c index 42317a2..fc16e5f 100644 --- a/memory.c +++ b/memory.c @@ -914,7 +914,6 @@ void memory_region_init(MemoryRegion *mr, if (size == UINT64_MAX) { mr->size = int128_2_64(); } - mr->name = g_strdup(name); if (name) { object_property_add_child_array(owner, name, OBJECT(mr)); @@ -1309,6 +1308,10 @@ uint64_t memory_region_size(MemoryRegion *mr) const char *memory_region_name(const MemoryRegion *mr) { + if (!mr->name) { + ((MemoryRegion *)mr)->name = + object_get_canonical_path_component(OBJECT(mr)); + } return mr->name; }
To support name retrieval of MemoryRegions that were created dynamically (that is, not via memory_region_init and friends). We cache the name in MemoryRegion's state as object_get_canonical_path_component mallocs the returned value so it's not suitable for direct return to callers. Memory already frees the name field, so this will be garbage collected along with the MR object. Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> --- memory.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)