From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40497) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XpHoW-0003oZ-GV for qemu-devel@nongnu.org; Fri, 14 Nov 2014 09:25:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XpHoN-0001rV-Ic for qemu-devel@nongnu.org; Fri, 14 Nov 2014 09:25:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56112) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XpHoN-0001rN-Al for qemu-devel@nongnu.org; Fri, 14 Nov 2014 09:25:19 -0500 Message-ID: <546610C9.3060706@redhat.com> Date: Fri, 14 Nov 2014 15:25:13 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1415812130-2592-1-git-send-email-armbru@redhat.com> <54639524.3060205@redhat.com> <87wq6zmsiz.fsf@blackfin.pond.sub.org> <54648333.2050808@redhat.com> <87sihn9nji.fsf@blackfin.pond.sub.org> <5464C29D.6090706@redhat.com> <87a93t3lsd.fsf@blackfin.pond.sub.org> In-Reply-To: <87a93t3lsd.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/4] qom: Replace "automatic arrayification" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: peter.crosthwaite@xilinx.com, qemu-devel@nongnu.org, afaerber@suse.de 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