From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54201) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RW6zE-000872-Lp for qemu-devel@nongnu.org; Thu, 01 Dec 2011 08:47:46 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RW6z8-0003Wg-GM for qemu-devel@nongnu.org; Thu, 01 Dec 2011 08:47:40 -0500 Received: from mail-gy0-f173.google.com ([209.85.160.173]:42992) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RW6z8-0003Wc-CU for qemu-devel@nongnu.org; Thu, 01 Dec 2011 08:47:34 -0500 Received: by ghbg19 with SMTP id g19so2355838ghb.4 for ; Thu, 01 Dec 2011 05:47:34 -0800 (PST) Message-ID: <4ED78573.508@codemonkey.ws> Date: Thu, 01 Dec 2011 07:47:31 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1322687028-29714-1-git-send-email-aliguori@us.ibm.com> <1322687028-29714-8-git-send-email-aliguori@us.ibm.com> <4ED76325.6080305@redhat.com> <4ED7744D.1080306@redhat.com> In-Reply-To: <4ED7744D.1080306@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 07/18] qom: add link properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: Kevin Wolf , Peter Maydell , Anthony Liguori , Stefan Hajnoczi , Stefan Hajnoczi , qemu-devel@nongnu.org, Markus Armbruster , Jan Kiszka , Luiz Capitulino , Michael Roth On 12/01/2011 06:34 AM, Avi Kivity wrote: > On 12/01/2011 01:35 PM, Stefan Hajnoczi wrote: >>>> >>>> +static void qdev_get_link_property(DeviceState *dev, Visitor *v, void *opaque, >>>> + const char *name, Error **errp) >>>> +{ >>>> + DeviceState **child = opaque; >>>> + gchar *path; >>>> + >>>> + if (*child) { >>>> + path = qdev_get_canonical_path(*child); >>>> + visit_type_str(v,&path, name, errp); >>>> + g_free(path); >>>> + } else { >>>> + path = (gchar *)""; >>> >>> If gchar != char, this is wrong. Also, you're converting a const >>> pointer into a non-const pointer, discarding type safety. >> >> This looked weird to me too but the cast has to do with the fact that >> the visitor function works both for input and output visitors. The >> output visitor needs to write to gchar** while the input visitor does >> not. > > So you need to pass a non-const pointer to an array of const char, or > const gchar **. You don't modify the string in place, you allocate a > new string and free the old one. > > >> When this function is called with the correct visitor type we are >> guaranteed that path will not be modified. > > What if it's called with the output visitor? (warning: confusing > convention). The reason there's a single Visitor type that works for both input and output visitors is to make it so you can write a single visit function that works for input and output. This works very well for most cases (in fact, QAPI makes heavy use of it). That said, I'm starting to feel like there should be a separate input and output visitor interface. It would make a number of things (like visiting lists) significantly simpler. Regards, Anthony Liguori