Re: [Qemu-devel] [PATCH 0/4] qom: Replace "automatic arrayification"

[Paolo Bonzini]

On 14/11/2014 15:21, Markus Armbruster wrote:
> qdev properties can easily use object_gen_new_property_name() instead,
> as shown in PATCH 3/4.
> 
> So can memory regions [PATCH 2/4], but only because we clumsily arrayify
> all of them, by appending "[*]" in memory_region_init().
> 
> Some day, we may want to arrayify only the ones that actually need it,
> by appending "[*]" right to their names instead of appending it behind
> the scenes to all memory region names.
> 
> This would involve touching the untouchables: non-qdevified devices.
> But the changes should be limited to string literals.
> 
> With my series, you'd have to graft in object_gen_new_property_name()
> and the matching g_free() instead.  You called that "just too ugly".
> 
> Here's how to avoid it, and confine the ugliness to
> memory_region_init():
> 
> Change memory_region_init() from
> 
>         char *escaped_name = memory_region_escape_name(name);
>         char *propname = object_gen_new_property_name(owner, escaped_name);
>         object_property_add_child(owner, propname, OBJECT(mr), &error_abort);
>         object_unref(OBJECT(mr));
>         g_free(propname);
>         g_free(escaped_name);
> 
> to something like
> 
>         if (name ends with "[*]") {
>             stem = g_strndup(name, strlen(name) -3);
>             escaped = memory_region_escape_name(stem);
>             propname = object_gen_new_property_name(owner, escaped_name);
>             g_free(escaped);
>             g_free(stem);
>         else
>             propname = memory_region_escape_name(name);
>         object_property_add_child(owner, propname, OBJECT(mr), &error_abort);
>         object_unref(OBJECT(mr));
>         g_free(propname);
> 
> and append "[*]" to the names of regions that need "arrayification".
> 
> That way, the bad magic is limited to just memory_region_init() rather
> than all of QOM, and it's needed "only" as long as the problem with
> non-qdevified users remains.

Yes, I agree.

Whether it's an improvement to move back the special-casing of "[*]" to
memory_region_init(), that's of course in the eye of the beholder. ;)
We do know that it's very unlikely to be removed from memory regions.

Paolo