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

[Markus Armbruster <armbru@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 13/11/2014 15:34, Markus Armbruster wrote:
>> -    ... "bla" ...
>> +    char *name = object_gen_new_property_name(obj, "bla");
>> +    ... name ...
>> +    g_free(name);
>> 
>> Not quite as mechanical, in particular finding the obj to pass to
>> object_gen_new_property_name().
>> 
>> Correct?
>
> Yes (except that the obj is just qdev_get_machine()).

Ah, of course.

Let me summarize.  "Automatic arrayification" currently has two kinds of
users: qdev properties and memory regions.

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.

>> [*] What we want to do is drag them along until the Second Coming, when
>> Jesus will set everything right, which naturally includes qdevifying all
>> the old crap nobody wants to touch.
>
> Or maybe not.  Look at the bottom:
> https://upload.wikimedia.org/wikipedia/commons/a/a5/Michelangelo%2C_Giudizio_Universale_02.jpg

Oww!  Well played, sir!