From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:41236) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RW731-0001al-KA for qemu-devel@nongnu.org; Thu, 01 Dec 2011 08:51:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RW72v-00041Y-Nu for qemu-devel@nongnu.org; Thu, 01 Dec 2011 08:51:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47900) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RW72v-00041R-DR for qemu-devel@nongnu.org; Thu, 01 Dec 2011 08:51:29 -0500 Message-ID: <4ED78642.6040208@redhat.com> Date: Thu, 01 Dec 2011 15:50:58 +0200 From: Avi Kivity 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> <4ED78573.508@codemonkey.ws> In-Reply-To: <4ED78573.508@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1 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: Anthony Liguori 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 03:47 PM, Anthony Liguori wrote: > 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. Perhaps. But that's independent of the issue here. No matter how many visitors you have, you never change a gchar in a string in place, you always allocate a new string (or you can't change the string length). So the type needs to be const gchar **, not gchar **. -- error compiling committee.c: too many arguments to function